Skip to content

Conversation

@plammens
Copy link
Contributor

@plammens plammens commented Dec 14, 2020

The only public way to access the correct config object seems to be through the pytestconfig fixture, however I don't know how to get a hold of that outside a test function. So I used get_config from _pytest.config. No idea whether this is the correct approach but at least the test passes 😅


Update:

After taking a walk through pytest code with the company of a debugger, it seems the point at which the config object is constructed is here:

https://github.com/pytest-dev/pytest/blob/ad65e816e4612af4413711200f9e58d74e765a3a/src/_pytest/config/__init__.py#L143

Hence probably _prepare_config is the way to go? For now this PR adds a cached lazy _get_config static method to MiniMetafunc that calls _prepare_config. Ideally though it would be best to reuse the config object that pytest has already computed, but I have no idea how to get that from the MiniMetafunc code.

Fixes #165

@codecov-io
Copy link

codecov-io commented Dec 14, 2020

Codecov Report

Merging #166 (246ba8a) into master (600b294) will decrease coverage by 0.02%.
The diff coverage is 75.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #166      +/-   ##
==========================================
- Coverage   91.22%   91.19%   -0.03%     
==========================================
  Files         130      131       +1     
  Lines        5025     5031       +6     
==========================================
+ Hits         4584     4588       +4     
- Misses        441      443       +2     
Impacted Files Coverage Δ
pytest_cases/tests/cases/issues/test_issue_165.py 60.00% <60.00%> (ø)
pytest_cases/common_pytest.py 88.46% <100.00%> (+0.03%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 600b294...56330d7. Read the comment docs.

@plammens plammens force-pushed the fix/165-empty-parameter-set branch 3 times, most recently from 71624c2 to 56330d7 Compare December 15, 2020 16:10
@smarie
Copy link
Owner

smarie commented Dec 16, 2020

Thanks @plammens for this PR !

I did not yet bother using the config object for now, it should be actually used everywhere where I generate ids instead of pytest (my own mini_idvalset and mini_idval methods in common_pytest.py).

So I wonder, maybe there is a way to get this globally instead of hacking in the pytest internals ?

  • the easiest way to get the config seems to simply add this hook in our plugin.py: https://docs.pytest.org/en/stable/reference.html#pytest.hookspec.pytest_configure . Actually I already added it :) so it can be as simple as storing the object in a module variable when the hook is called, so as to be accessed later by MiniMetaFunc. Do you wish to try this ?

  • there seem to be a fixture available https://docs.pytest.org/en/stable/reference.html#pytestconfig, but not sure it was there in pytest 2 so we'll need to backport it (or not support it in old versions). EDIT: actually it is not possible to access a fixture from the inside of a decorator call (since fixtures are not yet initialized), so this is probably not the way to go.

@plammens
Copy link
Contributor Author

That seems way better, I'll take a look!

@plammens plammens force-pushed the fix/165-empty-parameter-set branch from 56330d7 to 59eff0b Compare December 18, 2020 11:39
@plammens plammens force-pushed the fix/165-empty-parameter-set branch from 59eff0b to 75c3a93 Compare December 18, 2020 11:41
@plammens plammens marked this pull request as ready for review December 18, 2020 11:44
@plammens
Copy link
Contributor Author

Allright, I used your suggestion above: when the pytest_configure hook is called, I set a global module attribute PYTEST_CONFIG which is later retrieved by MiniMetafunc. Take a look to see if this is what you intended, @smarie! Also rebased onto the latest master.

Copy link
Owner

@smarie smarie left a comment

Choose a reason for hiding this comment

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

Perfect ! Nicely done, thanks @plammens

@smarie smarie merged commit a9083f2 into smarie:master Dec 18, 2020
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.

Using an empty parameter set causes an error

3 participants