Skip to content

Fix/address flaky tests #289

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

Merged
seddonym merged 4 commits into from
Dec 20, 2025
Merged

Fix/address flaky tests #289

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
0d312d3
Use more robust path handling in `test_syntax_error_includes_module` …
nathanjmcdougall Dec 18, 2025
89e1039
Use context manager for settings in `configure_unit_tests` fixture
nathanjmcdougall Dec 18, 2025
36c2f39
Merge remote-tracking branch 'upstream/main' into fix/address-flaky-t…
nathanjmcdougall Dec 19, 2025
94e4421
Add `match` arg in `pytest.warns` for `DeprecationWarning` of `Names…
nathanjmcdougall Dec 19, 2025
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
9 changes: 5 additions & 4 deletions tests/functional/test_error_handling.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,14 +1,15 @@
import os
from pathlib import Path

import pytest # type: ignore

from grimp import build_graph, exceptions


def test_syntax_error_includes_module():
dirname = os.path.dirname(__file__)
filename = os.path.abspath(
os.path.join(dirname, "..", "assets", "syntaxerrorpackage", "foo", "one.py")
filename = str(
Copy link
Collaborator

@seddonym seddonym Dec 19, 2025

Choose a reason for hiding this comment

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

Out of interest, why does this change fix the issue? They look equivalent to me.

Copy link
Contributor Author

@nathanjmcdougall nathanjmcdougall Dec 19, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

There are differences in behaviour between os.path.abspath and pathlib.resolve(). Specifically (in this case) the latter will normalize Windows drive letter names to capitalize them, e.g. c:\ -> C:\. There are differences with unix too: abspath will only make the path absolute, e.g. it won't apply resolutions (e.g. symlinks etc.).

It looks like build_graph already applies this normalization, so to get the test to match, we need to match the exact string including capitalization.

Without the fix:

__file__ == 'c:\\Users\\namc\\Repositories\\grimp\\tests\\functional\\test_error_handling.py'
filename == 'c:\\Users\\namc\\Repositories\\grimp\\tests\\assets\\syntaxerrorpackage\\foo\\one.py'

Versus with the fix

__file__ == 'c:\\Users\\namc\\Repositories\\grimp\\tests\\functional\\test_error_handling.py'
filename == 'C:\\Users\\namc\\Repositories\\grimp\\tests\\assets\\syntaxerrorpackage\\foo\\one.py'

When running pytest from the CLI/Just, the VS Code console uses capital C:\ for the current directory (so tests pass) but when running from the VS Code test-running UI, by default the --root-dir argument is set using a non-normalized path with lower-case c:\, and the test was failing.

['-p', 'vscode_pytest', '--rootdir=c:\\Users\\namc\\Repositories\\grimp', '--capture=no', 'c:\\Users\\namc\\Repositories\\grimp\\tests\\functional\\test_error_handling.py::test_syntax_error_includes_module']

(
Path(__file__).parent.parent / "assets" / "syntaxerrorpackage" / "foo" / "one.py"
).resolve()
)

with pytest.raises(exceptions.SourceSyntaxError) as excinfo:
Expand Down
7 changes: 4 additions & 3 deletions tests/unit/conftest.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,14 +1,15 @@
import pytest # type: ignore

from grimp.application.config import settings
from grimp.application.graph import ImportGraph
from grimp.adaptors.modulefinder import ModuleFinder
from tests.config import override_settings


@pytest.fixture(scope="module", autouse=True)
def configure_unit_tests():
settings.configure(
with override_settings(
IMPORT_GRAPH_CLASS=ImportGraph,
MODULE_FINDER=ModuleFinder(),
FILE_SYSTEM=None,
)
):
yield
2 changes: 1 addition & 1 deletion tests/unit/test_exceptions.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -60,5 +60,5 @@ def test_different_texts_are_not_equal(self):

class TestNamespacePackageEncountered:
def test_deprecated_access(self):
with pytest.warns(DeprecationWarning):
with pytest.warns(DeprecationWarning, match="NamespacePackageEncountered is deprecated"):
exceptions.NamespacePackageEncountered