Skip to content

Add plugin for cpu/memory intensive tests - #48

Merged
pllim merged 4 commits into
astropy:mainfrom
saimn:intensive-tests
Feb 10, 2022
Merged

pllim merged 4 commits into
astropy:mainfrom
saimn:intensive-tests

Conversation

@saimn

@saimn saimn commented Jan 21, 2022

Copy link
Copy Markdown
Contributor

Related to astropy/astropy#12350

With this plugin, cpu/memory intensive tests

  • are not run by default
  • can be run with the other tests with --run-slow and --run-hugemem
  • can be run separately with -m slow and -n hugemem

If you have better ideas for the marker names ...

@saimn
saimn requested review from a team, astrofrog and pllim January 21, 2022 09:41

@pllim pllim left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I think these two new options need to be documented on README.

  • When should one mark a test as slow or hugemem or both?
  • How do you use these features?
  • What if another plugin already registered these markers? Then what is the intended behavior? (Possible since "slow" is actually in official pytest example.)

@pllim pllim added this to the v0.10.0 milestone Jan 21, 2022
@pllim

pllim commented Jan 21, 2022

Copy link
Copy Markdown
Member

Needs a change log too. Thanks!

@pllim pllim added the Build wheels Kick off Release workflow for PR label Jan 21, 2022
@pllim

pllim commented Jan 21, 2022

Copy link
Copy Markdown
Member

Maybe tests too or is that overkill? 🤷

@saimn

saimn commented Jan 21, 2022

Copy link
Copy Markdown
Contributor Author

When should one mark a test as slow or hugemem or both?

  • slow: for tests that take a lot of time and that you want to run only in one job, or in a specific job

What if another plugin already registered these markers? Then what is the intended behavior? (Possible since "slow" is actually in official pytest example.)

No idea, but not it's an issue for all plugins I guess.

Maybe tests too or is that overkill? shrug

Ideally yes but I'm not sure how to do that. It works on the astropy PR so that's a good test for now :)

@adrn

adrn commented Jan 24, 2022

Copy link
Copy Markdown
Member

I agree with @pllim that any new functionality here should be documented!

I'm also unsure on what makes the most sense: having a --run-slow option (where by default slow tests are skipped), or having a --skip-slow option (where by default all tests are run). Do you have thoughts on that?

@saimn

saimn commented Jan 24, 2022

Copy link
Copy Markdown
Contributor Author

Sure I can add some docs, just wanted to get the feelings before since names etc could change.
The idea of --run-slow is mostly about defaults. We could also not have this option and use marker selection (-m not slow or something like that). But with the little plugin and --run-slow at least we can be sure that slow tests are not run by default which I think is what we want.

@pllim

pllim commented Jan 25, 2022

Copy link
Copy Markdown
Member

we can be sure that slow tests are not run by default

I agree. I wouldn't want to run them by default myself.

@pllim pllim left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Let's get this in so we can use it at astropy/astropy#12764 . Thanks!

@pllim
pllim merged commit 7ea7bec into astropy:main Feb 10, 2022
@saimn
saimn deleted the intensive-tests branch February 11, 2022 20:42
@eerovaher

Copy link
Copy Markdown
Member

This pull request is missing its change log entry.

@pllim

pllim commented Mar 3, 2022

Copy link
Copy Markdown
Member

Ops, thanks for catching that. Added in 2d4ea39 .

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Build wheels Kick off Release workflow for PR enhancement

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants