Skip to content
This repository has been archived by the owner on Oct 1, 2024. It is now read-only.

Add CLI command to show fixture information #154

Merged
merged 8 commits into from
May 1, 2020
Merged

Add CLI command to show fixture information #154

merged 8 commits into from
May 1, 2020

Conversation

JoshKarpel
Copy link
Contributor

(I was up too late and decided to take a crack at #151 ; apologies for not poking that issue in advance. This is my first time looking at Ward, so please let me know if this implementation is just bogus given other design considerations!)

This PR adds a CLI option --fixtures that a) prevents tests from executing at all (it does not even print them like in a dry run) and b) prints out information on all defined fixtures. The output from ward's own suite looks like this (I took the liberty of turning an unrelated comment in a fixture into a docstring to show what it would look like):

image

The simplest way to wire this up was to store off fixture functions to a module-level list in every invocation of the fixture decorator. This goes against the "don't look at fixtures until we run tests" note in #151 . However, it could enable other run options where Ward does this collection step, then runs the tests and print out more annotations on the fixtures, namely which/how many tests actually used them. I see some value in this kind of lightweight introspection to identify all defined fixtures, regardless of whether they're used or not (and it still obeys the less-strict statement "only look at fixtures that are imported by the test suite").

No tests yet, because I wasn't sure how you would want this kind of CLI output tested.

@darrenburns
Copy link
Owner

This looks fantastic! Thanks so much Josh 🙂 I'll try to review it ASAP!

Copy link
Owner

@darrenburns darrenburns left a comment

Choose a reason for hiding this comment

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

What happens in the case a fixture depends on a fixture which depends on a fixture?

I've put some initial comments/suggestions. Let me know what you think 😄

ward/terminal.py Outdated Show resolved Hide resolved
ward/fixtures.py Outdated Show resolved Hide resolved
ward/terminal.py Outdated Show resolved Hide resolved
ward/run.py Outdated Show resolved Hide resolved
@JoshKarpel JoshKarpel marked this pull request as draft April 26, 2020 18:17
… information into a "fixtures" subcommand.

Currently broken by pallets/click#430 (but might be resolved by pallets/click#1540)
@JoshKarpel
Copy link
Contributor Author

So it turns out the cleanest way to effectively alias ward -> ward test (993ea28#diff-dc25b79eb179f88480d4a5865ee54500R30) is broken because of pallets/click#430, but might be fixed if pallets/click#1540 makes it into a Click release. There are hackier ways to do it if waiting is undesirable.

@thebigmunch
Copy link
Contributor

So it turns out the cleanest way to effectively alias ward -> ward test (993ea28#diff-dc25b79eb179f88480d4a5865ee54500R30) is broken because of pallets/click#430, but might be fixed if pallets/click#1540 makes it into a Click release. There are hackier ways to do it if waiting is undesirable.

Would https://github.com/click-contrib/click-default-group not be an option for this? Also marks the default command in the help.

@JoshKarpel
Copy link
Contributor Author

So it turns out the cleanest way to effectively alias ward -> ward test (993ea28#diff-dc25b79eb179f88480d4a5865ee54500R30) is broken because of pallets/click#430, but might be fixed if pallets/click#1540 makes it into a Click release. There are hackier ways to do it if waiting is undesirable.

Would https://github.com/click-contrib/click-default-group not be an option for this? Also marks the default command in the help.

If it's alright to add the dependency, that would certainly work.

@darrenburns
Copy link
Owner

So it turns out the cleanest way to effectively alias ward -> ward test (993ea28#diff-dc25b79eb179f88480d4a5865ee54500R30) is broken because of pallets/click#430, but might be fixed if pallets/click#1540 makes it into a Click release. There are hackier ways to do it if waiting is undesirable.

Would https://github.com/click-contrib/click-default-group not be an option for this? Also marks the default command in the help.

If it's alright to add the dependency, that would certainly work.

I wouldn't be against adding that dependency (but the alias is a pretty minor thing IMO.)

@JoshKarpel JoshKarpel marked this pull request as ready for review April 26, 2020 21:59
@JoshKarpel JoshKarpel changed the title Add CLI option to show fixture information Add CLI command to show fixture information Apr 26, 2020
@darrenburns darrenburns self-assigned this Apr 26, 2020
ward/run.py Outdated Show resolved Hide resolved
ward/run.py Outdated Show resolved Hide resolved
Copy link
Owner

@darrenburns darrenburns left a comment

Choose a reason for hiding this comment

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

This is fantastic. I made a couple of very small text changes/updates but looks good 👍 !

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants