search

gristlabs / asttokens

Annotate Python AST trees with source text and token information
Apache License 2.0
172 stars 34 forks source link

Run slow tests without coverage #132

Closed PeterJCLaw closed 1 year ago

PeterJCLaw commented 1 year ago

Per suggestion from @alexmojaki at https://github.com/gristlabs/asttokens/issues/129#issuecomment-1779132057

These slower tests are integration/kitchen-sink tests, so they shouldn't add substantially to the code coverage. Running them without coverage is quite a lot faster (several minutes in CI) which is well worth the small additional complexity.

This commit also drops pytest outputting the junit xml file as there's not an obvious way to combine that from multiple test runs and it appears to be unused.

This reduces the CI time from ~20 minutes to ~10 minutes, however there's a small reduction in measured coverage. Happy to address that here with unit tests if desired.

Builds on #128. Fixes #129 as CI time is no longer quite as silly.

alexmojaki commented 1 year ago

Please don't @ me in commit messages, it does weird things with github notifications

PeterJCLaw commented 1 year ago

Oh, sorry, hadn't thought about that. Yeah, I suspect it combined badly with force pushing a few times to fix CI.

alexmojaki commented 1 year ago

Hmmm, I feel weird about having two mechanisms for dealing with slow tests: ASTTOKENS_SLOW_TESTS and the pytest marker.

PeterJCLaw commented 1 year ago

Hmmm, I feel weird about having two mechanisms for dealing with slow tests: ASTTOKENS_SLOW_TESTS and the pytest marker.

Ah, good point. Do you think that having some of the sys.modules tests in the normal suite adds value? If not, we could just push them entirely into the slow suite and drop ASTTOKENS_SLOW_TESTS. On the other hand doing that would mean that locally the default would be to run all the tests, which might not be ideal for rapid iteration. A script which just runs the fast subset might be useful to help there? (Unless there's a way to default pytest to not running a particular mark?)

PeterJCLaw commented 1 year ago

Unless there's a way to default pytest to not running a particular mark?

There is! We can add -m 'not slow' to the adopts key in setup.cfg. That can still be overridden on the command line (-m '' to run everything, -m slow to run just the slow ones), so that feels like it's probably ideal here.

Concretely then I'm proposing:

PeterJCLaw commented 1 year ago

Have implemented that approach in 52e9b4134ae42818e1dc386af6d9c9905e565016.

alexmojaki commented 1 year ago

This looks great, thank you!