Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add Windows to CI (plus associated fixes) #287

Merged
merged 19 commits into from
Dec 7, 2020
Merged

Add Windows to CI (plus associated fixes) #287

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
19 commits
Select commit Hold shift + click to select a range
e97c46e
Matrix against Linux/macOS/Windows
altendky Dec 3, 2020
1759685
Use -m twisted.trial in tox.ini for Windows
altendky Dec 4, 2020
ff26db8
fix for Windows
altendky Dec 4, 2020
01a210e
use io.open() for encoding= for py2 compatibility
altendky Dec 4, 2020
d7dc7e2
os.path.normpath instead of os.path.sep
altendky Dec 4, 2020
224d1e8
macOS will be added separately now
altendky Dec 4, 2020
00f3c0c
Merge branch 'master' into test_more_operating_systems
altendky Dec 5, 2020
c974f05
Add 287.misc.rst
altendky Dec 6, 2020
0a2d65c
Merge branch 'master' into test_more_operating_systems
altendky Dec 6, 2020
51b43c5
Explain git checkout -b master
altendky Dec 6, 2020
4314093
Explain coverage run --module twisted.trial
altendky Dec 6, 2020
5d3d920
Update test_check.py
altendky Dec 6, 2020
96d490f
simplify
altendky Dec 6, 2020
411dcd2
exclude 3.9 on windows due to trial (twisted)
altendky Dec 6, 2020
daf3d01
comment on why windows/cp39 are excluded from ci
altendky Dec 6, 2020
9fb95e4
Expound upon comment about 3.9 on windows
altendky Dec 6, 2020
0f049d6
Remove explicit setting of branch as master
altendky Dec 6, 2020
e20a790
adjust coverage run call and comment
altendky Dec 7, 2020
55f4966
Update tox.ini
altendky Dec 7, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 10 additions & 0 deletions .github/workflows/ci.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,8 @@ jobs:
runs-on: ubuntu-latest
- name: macOS
runs-on: macos-latest
- name: Windows
runs-on: windows-latest
python:
- name: CPython 2.7
tox: py27
Expand Down Expand Up @@ -46,6 +48,14 @@ jobs:
task:
- name: Test
tox: tests
exclude:
# Twisted and thus trial do not work on Windows with CPython 3.9.
# This will be fixed with the next release.
# https://twistedmatrix.com/trac/ticket/10027
- python:
tox: py39
os:
name: Windows

steps:
- uses: actions/checkout@v2
Expand Down
5 changes: 3 additions & 2 deletions src/towncrier/_writer.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@

from __future__ import absolute_import, division

import io
import os


Expand All @@ -21,8 +22,8 @@ def append_to_newsfile(
if not os.path.exists(news_file):
existing_content = u""
else:
with open(news_file, "rb") as f:
existing_content = f.read().decode("utf8")
with io.open(news_file, "r", encoding="utf8") as f:
existing_content = f.read()
existing_content = existing_content.split(start_line, 1)
else:
existing_content = [u""]
Expand Down
17 changes: 8 additions & 9 deletions src/towncrier/check.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,12 +48,10 @@ def __main(comparewith, directory, config):
click.echo("On trunk, or no diffs, so no newsfragment required.")
sys.exit(0)

files = set(
map(
lambda x: os.path.join(base_directory, x),
files_changed.strip().split(os.linesep),
)
)
files = {
os.path.normpath(os.path.join(base_directory, path))
for path in files_changed.strip().splitlines()
}

click.echo("Looking at these files:")
click.echo("----")
Expand All @@ -72,14 +70,15 @@ def __main(comparewith, directory, config):
)
fragment_directory = "newsfragments"

fragments = set(
find_fragments(
fragments = {
os.path.normpath(path)
for path in find_fragments(
fragment_base_directory,
config["sections"],
fragment_directory,
config["types"],
)[1]
)
}
fragments_in_branch = fragments & files

if not fragments_in_branch:
Expand Down
Empty file added src/towncrier/newsfragments/287.misc.rst
View file
Open in desktop
Empty file.
5 changes: 4 additions & 1 deletion src/towncrier/test/test_settings.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -169,7 +169,10 @@ def test_missing_template(self):
load_config(temp)

self.assertEqual(
str(e.exception), "The template file '%s/foo.rst' does not exist." % (temp,)
str(e.exception),
"The template file '{}' does not exist.".format(
os.path.normpath(os.path.join(temp, "foo.rst")),
)
)

def test_missing_template_in_towncrier(self):
Expand Down
4 changes: 3 additions & 1 deletion tox.ini
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,9 @@ commands =
coverage --version
{envbindir}/trial --version
coverage erase
coverage run -p {envbindir}/trial {posargs:towncrier}
# `coverage run` tries to act like Python so we use `--module` instead of
# specifying the entry point script in `{envbindir}`.
coverage run -p --module twisted.trial {posargs:towncrier}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

May not make any difference but this could also be python -m coverage .... I expect the right command is on PATH thanks to tox ... but a few lines above it looks like you thought python -m twisted.trial ... was a good idea, so maybe it's a good idea for coverage too?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The -m twisted.trial was for consistency with this line which switched as explained in the comment. But sure, could do it for everything. I'll look, or maybe another PR to switch everything if there are several unrelated places where -m would make sense.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

https://tox.readthedocs.io/en/latest/config.html#conf-allowlist_externals

Hmm, so tox only warns about outside access and doesn't, right there anyways, list an option to make that an error. Anyways, there are several other places this would apply in general so I won't change them here. I did update the comment to better represent coverage's perspective on the run subcommand.

coverage combine -a

[testenv:cov-report]
Expand Down