Make tests independent of optdeps

[hseg]

Description

test_content_negotiation_bad_bibtex as currently written will always run, even
if the optional dependency bibtexparser is not installed.

For people using the testsuite to validate their installs who aren't installing
bibtexparser, that makes this unnecessarily annoying (one needs to pass -k 'not test_...' to achieve the same effect).

Instead, this PR makes the test conditional on bibtexparser being installed.

Related Issue

Fixes: #144

[sckott]

Thanks @hseg

I think you might be using importorskip incorrectly? https://stackoverflow.com/questions/30086558/py-test-fails-due-to-missing-module https://docs.pytest.org/en/stable/how-to/skipping.html#skipping-on-a-missing-import-dependency

[hseg]

Huh, it worked for me, but that might've been a lucky coincidence - I'm guessing that pytest just abandons the testcase when it hits that line, so when I decorated the function with that it limited it to just that testcase? Or alternatively I didn't notice the skipping behaviour, just that it came out all green? A quick search indeed shows that this is unsupported behavior pytest-dev/pytest#9548 pytest-dev/pytest#9542 I suppose the correct way of writing this is using pytest.skipif wrapping a try-import statement. I'm afk, I'll see what I can do about this tomorrow. 22 ene 2025 20:00:15 Scott Chamberlain ***@***.***>:

…

Thanks @hseg[https://github.com/hseg] I think you might be using *importorskip* incorrectly? https://stackoverflow.com/questions/30086558/py-test-fails-due-to-missing-module https://docs.pytest.org/en/stable/how-to/skipping.html#skipping-on-a-missing-import-dependency — Reply to this email directly, view it on GitHub[#199 (comment)], or unsubscribe[https://github.com/notifications/unsubscribe-auth/AAJTW5ORCEEUC2DGEGFFZK32L7MC5AVCNFSM6AAAAABVVPYELWVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDMMBXHEYDSOJVHA]. You are receiving this because you were mentioned. [Imagen de rastreo][https://github.com/notifications/beacon/AAJTW5KPQQWEMRI45BWGGHT2L7MC5A5CNFSM6AAAAABVVPYELWWGG33NNVSW45C7OR4XAZNMJFZXG5LFINXW23LFNZ2KUY3PNVWWK3TUL5UWJTU3OGGEM.gif]

[hseg]

OK, so a try-except block was illegal there, and pkgutil is deprecated -- this is the solution I found.

[hseg]

(The need to run the test in a chroot in which bibtexparser is not installed makes my feedback loop go through pushing here, this should be the last of the changes. Sorry for the noise.)

[hseg]

I just realized that the test I put in is not enough -- it'll make the test fire
on any version of bibtexparser, where we need bibtexparser>=2.0.0b5.

This is a problem since papis requires both habanero and bibtexparser<2,
so I need pytest not to get confused in case bibtexparser is of a too-low
version.

[hseg]

Hrmph. The more I think on this, the deeper my concerns go -- this will actually
silence the fact that too-old bibtexparsers will cause the fix to cnrequest to
fail, so that needs fixing as well. Scope is creeping, sorry for dragging this
out. Oh goody, you already thought of that -- that feature is gated behind
major version 2 -- that's a load of frustration off my chest.

[sckott]

Sorry about the lack of tests running each commit - i was on the new "merge experience" UI here, and it wasn't showing the "Approve CI" button. Running now

[sckott]

Can you fix @hseg ?

[hseg]

Will be afk, possibly until Sunday, but will try to fix after.

[hseg]

Found it, forgot a stray paren. That should be it though.

[hseg]

Bds=bounds, I'll expand it. And yes, I added an upper bound in case we need to skip on too-new dependencies too. (The semantics I chose are to compare min<=ver<max because while you usually know your lower bound, the upper bound will be due to versions not yet released) 27 ene 2025 16:30:33 Scott Chamberlain ***@***.***>:

…

***@***.**** commented on this pull request. ---------------------------------------- In test/test-content_negotation.py[#199 (comment)]: > @@ -17,9 +17,26 @@ def test_content_negotiation(): assert isinstance(res, str) +def pkgverBds(pkg, minVer, maxVer=None): @hseg[https://github.com/hseg] can you explain what the *Bds* part of this function name refers to? And you put in a *maxVer* parameter in case it's needed later? — Reply to this email directly, view it on GitHub[#199 (review)], or unsubscribe[https://github.com/notifications/unsubscribe-auth/AAJTW5KEGGVQJSFZ4AL46U32MY7ITAVCNFSM6AAAAABVVPYELWVHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMZDKNZVGU4TONRUGA]. You are receiving this because you were mentioned. [Imagen de rastreo][https://github.com/notifications/beacon/AAJTW5LBLE2OYCE2BV7FK6L2MY7ITA5CNFSM6AAAAABVVPYELWWGG33NNVSW45C7OR4XAZNRKB2WY3CSMVYXKZLTORJGK5TJMV32UY3PNVWWK3TUL5UWJTUZQSAEQ.gif]