Skip to content

Commit

Permalink
🧪 Fix Windows pytest CI runs (#1153)
Browse files Browse the repository at this point in the history
Due to the way the Windows CLI works, the `coverage xml` command was overriding the exit code of `pytest` and so hiding its failures.
This commit fixes that and fixes the breaking tests

Co-authored-by: PhilipPartsch <[email protected]>
  • Loading branch information
chrisjsewell and PhilipPartsch authored Apr 11, 2024
1 parent 0f71ecd commit 90bae4a
Show file tree
Hide file tree
Showing 7 changed files with 47 additions and 36 deletions.
6 changes: 3 additions & 3 deletions .github/workflows/ci.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -44,9 +44,9 @@ jobs:
python -m pip install sphinx~=${{ matrix.sphinx-version }} -e .[test]
python -m pip freeze
- name: Run pytest
run: |
python -m pytest -v --ignore=tests/benchmarks -m "not jstest" --cov=sphinx_needs --cov-report=xml --cov-report=term-missing tests
coverage xml
run: python -m pytest -v --ignore=tests/benchmarks -m "not jstest" --cov=sphinx_needs --cov-report=xml --cov-report=term-missing tests
- name: Create coverage
run: coverage xml
- name: Upload to Codecov
if: github.repository == 'useblocks/sphinx-needs' && matrix.os == 'ubuntu-latest'
uses: codecov/codecov-action@v3
Expand Down
2 changes: 1 addition & 1 deletion tests/conftest.py
Original file line number Diff line number Diff line change
Expand Up @@ -289,7 +289,7 @@ def test_app(make_app, sphinx_test_tempdir, request):
# In this case we don't catch the warnings.
if builder_params.get("buildername", "html") == "html":
app.warning_list = strip_colors(
app._warning.getvalue().replace(str(app.srcdir), "srcdir")
app._warning.getvalue().replace(str(app.srcdir) + os.sep, "srcdir/")
).splitlines()
else:
app.warning_list = None
Expand Down
13 changes: 9 additions & 4 deletions tests/test_broken_links.py
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
from pathlib import Path

import pytest
from sphinx.util.console import strip_colors

Expand All @@ -12,9 +14,12 @@ def test_doc_build_html(test_app):
app.build()

# check there are expected warnings
warnings = strip_colors(app._warning.getvalue().replace(str(app.srcdir), "srcdir"))
warnings = strip_colors(app._warning.getvalue())
print(warnings.splitlines())
assert warnings.splitlines() == [
"srcdir/index.rst:12: WARNING: Need 'SP_TOO_002' has unknown outgoing link 'NOT_WORKING_LINK' in field 'links' [needs.link_outgoing]",
"srcdir/index.rst:21: WARNING: linked need BROKEN_LINK not found [needs.link_ref]",

expected_warnings = [
f"{Path(str(app.srcdir)) / 'index.rst'}:12: WARNING: Need 'SP_TOO_002' has unknown outgoing link 'NOT_WORKING_LINK' in field 'links' [needs.link_outgoing]",
f"{Path(str(app.srcdir)) / 'index.rst'}:21: WARNING: linked need BROKEN_LINK not found [needs.link_ref]",
]

assert warnings.splitlines() == expected_warnings
21 changes: 10 additions & 11 deletions tests/test_filter.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,19 +12,18 @@
def test_filter_build_html(test_app):
app = test_app
app.build()

warnings = strip_colors(
app._warning.getvalue().replace(str(app.srcdir), "srcdir")
).splitlines()
for w in warnings:
print(w)
assert warnings == [
"srcdir/index.rst:51: WARNING: Filter 'xxx' not valid. Error: name 'xxx' is not defined. [needs.filter]",
"srcdir/index.rst:54: WARNING: Filter '1' not valid. Error: Filter did not evaluate to a boolean, instead <class 'int'>: 1. [needs.filter]",
"srcdir/index.rst:57: WARNING: Filter 'yyy' not valid. Error: name 'yyy' is not defined. [needs.filter]",
"srcdir/index.rst:60: WARNING: Filter 'zzz' not valid. Error: name 'zzz' is not defined. [needs.filter]",
warnings = strip_colors(app._warning.getvalue())
print(warnings)

expected_warnings = [
f"{Path(str(app.srcdir)) / 'index.rst'}:51: WARNING: Filter 'xxx' not valid. Error: name 'xxx' is not defined. [needs.filter]",
f"{Path(str(app.srcdir)) / 'index.rst'}:54: WARNING: Filter '1' not valid. Error: Filter did not evaluate to a boolean, instead <class 'int'>: 1. [needs.filter]",
f"{Path(str(app.srcdir)) / 'index.rst'}:57: WARNING: Filter 'yyy' not valid. Error: name 'yyy' is not defined. [needs.filter]",
f"{Path(str(app.srcdir)) / 'index.rst'}:60: WARNING: Filter 'zzz' not valid. Error: name 'zzz' is not defined. [needs.filter]",
]

assert warnings.splitlines() == expected_warnings

html = Path(app.outdir, "index.html").read_text()
assert "story_a_1" in html
assert "story_b_1" not in html
Expand Down
8 changes: 5 additions & 3 deletions tests/test_github_issues.py
Original file line number Diff line number Diff line change
Expand Up @@ -34,11 +34,13 @@ def test_doc_github_44(test_app):
assert "Test 3" in html

stderr = output.stderr.decode("utf-8")
stderr = stderr.replace(str(app.srcdir), "srcdir")
assert stderr.splitlines() == [
"srcdir/index.rst:11: WARNING: Need 'test_3' has unknown outgoing link 'test_123_broken' in field 'links' [needs.link_outgoing]"

expected_warnings = [
f"{Path(str(app.srcdir)) / 'index.rst'}:11: WARNING: Need 'test_3' has unknown outgoing link 'test_123_broken' in field 'links' [needs.link_outgoing]"
]

assert stderr.splitlines() == expected_warnings


@pytest.mark.parametrize(
"test_app",
Expand Down
22 changes: 12 additions & 10 deletions tests/test_report_dead_links.py
Original file line number Diff line number Diff line change
Expand Up @@ -20,13 +20,14 @@ def test_needs_dead_links_warnings(test_app):

# check there are expected warnings
stderr = output.stderr.decode("utf-8")
stderr = stderr.replace(str(src_dir), "srcdir")
assert stderr.splitlines() == [
"srcdir/index.rst:17: WARNING: Need 'REQ_004' has unknown outgoing link 'ANOTHER_DEAD_LINK' in field 'links' [needs.link_outgoing]",
"srcdir/index.rst:45: WARNING: Need 'TEST_004' has unknown outgoing link 'REQ_005.invalid' in field 'links' [needs.link_outgoing]",
"srcdir/index.rst:45: WARNING: Need 'TEST_004' has unknown outgoing link 'REQ_005.invalid' in field 'tests' [needs.link_outgoing]",
expected_warnings = [
f"{Path(str(app.srcdir)) / 'index.rst'}:17: WARNING: Need 'REQ_004' has unknown outgoing link 'ANOTHER_DEAD_LINK' in field 'links' [needs.link_outgoing]",
f"{Path(str(app.srcdir)) / 'index.rst'}:45: WARNING: Need 'TEST_004' has unknown outgoing link 'REQ_005.invalid' in field 'links' [needs.link_outgoing]",
f"{Path(str(app.srcdir)) / 'index.rst'}:45: WARNING: Need 'TEST_004' has unknown outgoing link 'REQ_005.invalid' in field 'tests' [needs.link_outgoing]",
]

assert stderr.splitlines() == expected_warnings


@pytest.mark.parametrize(
"test_app",
Expand All @@ -44,13 +45,14 @@ def test_needs_dead_links_warnings_needs_builder(test_app):

# check there are expected warnings
stderr = output.stderr.decode("utf-8")
stderr = stderr.replace(str(src_dir), "srcdir")
assert stderr.splitlines() == [
"srcdir/index.rst:17: WARNING: Need 'REQ_004' has unknown outgoing link 'ANOTHER_DEAD_LINK' in field 'links' [needs.link_outgoing]",
"srcdir/index.rst:45: WARNING: Need 'TEST_004' has unknown outgoing link 'REQ_005.invalid' in field 'links' [needs.link_outgoing]",
"srcdir/index.rst:45: WARNING: Need 'TEST_004' has unknown outgoing link 'REQ_005.invalid' in field 'tests' [needs.link_outgoing]",
expected_warnings = [
f"{Path(str(app.srcdir)) / 'index.rst'}:17: WARNING: Need 'REQ_004' has unknown outgoing link 'ANOTHER_DEAD_LINK' in field 'links' [needs.link_outgoing]",
f"{Path(str(app.srcdir)) / 'index.rst'}:45: WARNING: Need 'TEST_004' has unknown outgoing link 'REQ_005.invalid' in field 'links' [needs.link_outgoing]",
f"{Path(str(app.srcdir)) / 'index.rst'}:45: WARNING: Need 'TEST_004' has unknown outgoing link 'REQ_005.invalid' in field 'tests' [needs.link_outgoing]",
]

assert stderr.splitlines() == expected_warnings


@pytest.mark.parametrize(
"test_app",
Expand Down
11 changes: 7 additions & 4 deletions tests/test_service_github.py
Original file line number Diff line number Diff line change
Expand Up @@ -84,13 +84,16 @@ def test_build(test_app, snapshot):

app = test_app
app.build()
warnings = strip_colors(app._warning.getvalue().replace(str(app.srcdir), "srcdir"))
warnings = strip_colors(app._warning.getvalue())
print(warnings)
assert warnings.splitlines() == [
'srcdir/index.rst:4: WARNING: "query" or "specific" missing as option for github service. [needs.github]',
"srcdir/index.rst:22: WARNING: GitHub: API rate limit exceeded (twice). Stop here. [needs.github]",

expected_warnings = [
f'{Path(str(app.srcdir)) / "index.rst"}:4: WARNING: "query" or "specific" missing as option for github service. [needs.github]',
f"{Path(str(app.srcdir)) / 'index.rst'}:22: WARNING: GitHub: API rate limit exceeded (twice). Stop here. [needs.github]",
]

assert warnings.splitlines() == expected_warnings

needs_data = json.loads((Path(app.outdir) / "needs.json").read_text("utf8"))
assert needs_data == snapshot(exclude=props("created", "avatar"))

Expand Down

0 comments on commit 90bae4a

Please sign in to comment.