Skip to content

Conversation

@Otto-AA
Copy link
Collaborator

@Otto-AA Otto-AA commented Apr 12, 2025

This change should help troubleshooting errors, where the package setup in ./mutants is not correct.

Currently, when the tests in ./mutants/ try to import a package that is not in ./mutants/, it will also look in the original (non-mutated) source code. Is there ever a case, where this is desired? I would guess that also_copy covers these use cases, but I am not sure, so I wanted to ask for your opinion @boxed .

The reason I want to prevent importing the original code is that it hides the error cause if the package setup of ./mutants is wrong. If the package is not setup properly (e.g. #370 , which only has a single file for paths_to_mutate rather than the whole source), the tests currently just use the original code and thus always pass. We only stop at the forced fail test.

With these changes, it will already fail at the test collecting stage. If debug=true, we also have the error cause reported:

ImportError while loading conftest '/.../narwhals/mutants/tests/conftest.py'.
../tests/conftest.py:14: in <module>
    from narwhals.utils import generate_temporary_column_name
E   ModuleNotFoundError: No module named 'narwhals.utils'
    exit code 4

This prevents that the tests accidentally import
the unmodified code, rather than the mutated code
from ./mutants/.

Now we directly raise an import error at tests collection,
rather than failing later on at the "forced fail test" run
because the tests succeed on the original (non-mutated) code.
@boxed
Copy link
Owner

boxed commented Apr 12, 2025

I like it!

@boxed boxed merged commit 1548d03 into main Apr 12, 2025
10 checks passed
@Otto-AA Otto-AA deleted the sys-path-removal branch April 14, 2025 06:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants