-
Notifications
You must be signed in to change notification settings - Fork 31
Fix test module imports #277
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.
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
Conversation
This commit modifies `top_level_dir`, which is used to resolve module names when loading unittests, to ensure, only modules from specified ST package are importet. Without this commit, UnitTesting might attempt to import packages/modules/tests from Lib/ directory due to possible ambiguities. Scenario: Plugins of `MyPackage` are organized in `Packages/MyPackage/dirname/` and tests in `Packages/MyPackage/dirname/tests/`. If a library `Lib/pythonXX/dirname` exists, Unittesting would have attempted to import tests from that directory, because test module paths would have been translated to `dirname.tests` due to `top_level_dir` pointing to `Packages/MyPackage`. By adjusting `top_level_dir` to `Packages/`, test module names are translated to `MyPackage.dirname.tests` instead.
88aec60 to
d3f1b00
Compare
|
Hello @deathaxe, FYI this commit seems to have broken really simple setups (UnitTesting documentation followed early 2025 for SSHubl).
The only "fix" that I've found so far is to :
WDYT ? Did we miss something in UnitTesting setup, or this is definitely an UnitTesting v1 regression ? |
|
The package (e.g. UnitTesting not respecting this difference was a mistake (or side-effect of former ST2 environments), which could have caused ambiguites with libraries using same name as the modules from a sub-package to test. If e.g. a library Assuming, tests are always executed by UnitTesting within ST plugin environment, a top-level As a consequence of this PR, test modules need to use relative imports (e.g.: Functions like |
|
Thanks for your detailed answer. Whereas it may actually fix some UnitTesting usages as #67, it's definitely a regression/breaking change for others. I've fixed our downstream very case, and as a side-note, a top Bye 👋 |
This commit clarifies how test cases are expected to import or address modules of a Sublime Text package as it slightly changed in PR #277.
|
This patch was tested against various popular ST packages (e.g.: GitSavvy, LSP, MarkdownEditing, NeoVintagious, etc.), none of which breaks. Admittedly all of them respect ST's Package/plugin infrastructure and therefore use absolute imports including package names. Hence it specifically just fixed an ambiguity with testing Package Control, which provides a Hence impact was considdered rather low. As a "breaking" change, it would have required major version to be bumbed, which however is not justified by just such a little change. Especially, as the required import and folder structure is not documented or specified anywhere. There are many packages which use tests correct. Those which don't respect ST3's package or plugin structure, may need adjustments. Edit: mypy needing a top-level |
This commit modifies
top_level_dir, which is used to resolve module names when loading unittests, to ensure, only modules from specified ST package are importet.Without this commit, UnitTesting might attempt to import packages/modules/tests from Lib/ directory due to possible ambiguities.
Scenario:
Plugins of
MyPackageare organized inPackages/MyPackage/dirname/and tests inPackages/MyPackage/dirname/tests/. If a libraryLib/pythonXX/dirnameexists, Unittesting would have attempted to import tests from that directory, because test module paths would have been translated todirname.testsdue totop_level_dirpointing toPackages/MyPackage.By adjusting
top_level_dirtoPackages/, test module names are translated toMyPackage.dirname.testsinstead.