Minor test cleanup #1090
Minor test cleanup #1090
Conversation
sirosen
requested review from
aaschaer,
ada-globus,
derek-globus,
kurtmckee,
m1yag1 and
MaxTueckeGlobus
as code owners
tests/unit/helpers/test_search.py
Outdated
| import globus_sdk | ||
| from globus_sdk import SearchQuery, SearchQueryV1, utils | ||
| from globus_sdk.exc.warnings import RemovedInV4Warning | ||
|
|
||
|
|
||
| @pytest.mark.filterwarnings("ignore:'SearchQuery'*:DeprecationWarning") | ||
| def test_init_legacy(): | ||
| """Creates SearchQuery and verifies results""" | ||
| query = SearchQuery() | ||
| with pytest.warns( | ||
| globus_sdk.exc.RemovedInV4Warning, match="'SearchQuery' is deprecated" | ||
| ): | ||
| query = SearchQuery() |
There was a problem hiding this comment.
These imports, and the subsequent usage here, aren't looking okay to me.
globus_sdk doesn't need to be imported, and the RemovedInV4Warning doesn't need to be qualified.
with pytest.warns(RemovedInV4Warning, match="'SearchQuery' is deprecated"):
query = SearchQuery()There was a problem hiding this comment.
Oh, 🤦 , I missed that it was already imported by name.
That said, we document it as globus_sdk.RemovedInV4Warning, so I think it's best if the testsuite matches that usage. (Which is also not what I wrote!)
I'll make a change right now which is fairly minimal. Although I generally prefer the verbose style of importing a package name (in our case, globus_sdk) and accessing all of the qualified names we need from that name, rewriting the whole test module to use that style isn't the small change I had in mind for this PR.
In the module where a `from globus_sdk import ...` is used, `RemovedInV4Warning` is imported there, inlining it with other imports and matching the documented usage for users. In the module where `import globus_sdk` is used, the warning is accessed as `globus_sdk.RemovedInV4Warning`. Again, this matches user-facing documentation. The two modules are not made consistent *with one another*, but they are now each internally consistent. --- Separately, fix a small issue which crept in, in which multiple tests were written in a single test function and I therefore failed to attach the requisite context manager.
📚 Documentation preview 📚: https://globus-sdk-python--1090.org.readthedocs.build/en/1090/