Skip to content

Use pytest markers for intensive tests - #12764

Merged
pllim merged 5 commits into
astropy:mainfrom
saimn:intensive-tests
Apr 22, 2022
Merged

Use pytest markers for intensive tests#12764
pllim merged 5 commits into
astropy:mainfrom
saimn:intensive-tests

Conversation

@saimn

@saimn saimn commented Jan 21, 2022 •
edited by pllim
Loading

Copy link
Copy Markdown
Contributor

DO NOT MERGE until tmp commit is removed.

Closes #12350. Depends on:

Checklist for package maintainer(s)

This checklist is meant to remind the package maintainer(s) who will review this pull request of some common things to look for. This list is not exhaustive.

  • Do the proposed changes actually accomplish desired goals?
  • Do the proposed changes follow the Astropy coding guidelines?
  • Are tests added/updated as required? If so, do they follow the Astropy testing guidelines?
  • Are docs added/updated as required? If so, do they follow the Astropy documentation guidelines?
  • Is rebase and/or squash necessary? If so, please provide the author with appropriate instructions. Also see "When to rebase and squash commits".
  • Did the CI pass? If no, are the failures related? If you need to run daily and weekly cron jobs as part of the PR, please apply the Extra CI label.
  • Is a change log needed? If yes, did the change log check pass? If no, add the no-changelog-entry-needed label. If this is a manual backport, use the skip-changelog-checks label unless special changelog handling is necessary.
  • Is a milestone set? Milestone must be set but astropy-bot check might be missing; do not let the green checkmark fool you.
  • At the time of adding the milestone, if the milestone set requires a backport to release branch(es), apply the appropriate backport-X.Y.x label(s) before merge.

@saimn saimn added this to the v5.1 milestone Jan 21, 2022
@saimn
saimn requested review from astrofrog and pllim January 21, 2022 10:21
@saimn
saimn marked this pull request as draft January 21, 2022 10:23
@pllim

This comment has been minimized.

Sign in to view
@pllim

pllim commented Feb 10, 2022

Copy link
Copy Markdown
Member

astropy/pytest-astropy#48 is merged but I am not sure if you need a pytest-astropy release first or you plan to develop this PR based on its dev version and only release when it is proven to work. Please let me know. Thanks!

@pllim pllim mentioned this pull request Feb 10, 2022
Closed
8 of 9 tasks
@saimn

saimn commented Feb 15, 2022

Copy link
Copy Markdown
Contributor Author

Yes we need a pytest-astropy release. It's already working with the dev version in this PR (I modified the 3.10 build to run only the slow tests, I should update that before merging).

@saimn
saimn force-pushed the intensive-tests branch from 0b4d908 to 526b9be Compare April 12, 2022 19:45
@saimn
saimn marked this pull request as ready for review April 12, 2022 19:46
@saimn
saimn force-pushed the intensive-tests branch from 799e6e2 to e7e1a0c Compare April 12, 2022 20:27
@nstarman

nstarman commented Apr 19, 2022

Copy link
Copy Markdown
Member

@saimn, what's the status of this PR, given the imminent feature freeze?

@saimn

saimn commented Apr 20, 2022

Copy link
Copy Markdown
Contributor Author

The PR is ready. I marked the tests taking more than ~5 sec as slow, so those are now run only in 2 builds.

@pllim

pllim commented Apr 20, 2022

Copy link
Copy Markdown
Member

@saimn , can you please rebase to pick up CI fixes and then I will slap the Extra CI label on this, just to be sure? Thanks!

@saimn
saimn force-pushed the intensive-tests branch from e7e1a0c to 46c63ba Compare April 20, 2022 13:13
@pllim pllim added the Extra CI Run cron CI as part of PR label Apr 21, 2022
@pllim

pllim commented Apr 21, 2022

Copy link
Copy Markdown
Member

@saimn , do you know why this broke pyinstaller?

@pllim

pllim commented Apr 21, 2022

Copy link
Copy Markdown
Member

Maybe you have to declare it here?

astropy/.pyinstaller/run_astropy_tests.py

Line 109 in 11f0dac

plugins=['pytest_doctestplus.plugin',

@saimn
saimn force-pushed the intensive-tests branch from 46c63ba to 53e91c1 Compare April 21, 2022 18:06
@saimn

saimn commented Apr 21, 2022

Copy link
Copy Markdown
Contributor Author

Maybe you have to declare it here?

Let's see if that works (thanks for finding it !).

@pllim

This comment was marked as resolved.

Sign in to view
@saimn
saimn force-pushed the intensive-tests branch from 03f2018 to 5e48db6 Compare April 22, 2022 08:11
@saimn

saimn commented Apr 22, 2022

Copy link
Copy Markdown
Contributor Author

@pllim - Squashed some of the commits.

@pllim

pllim commented Apr 22, 2022

Copy link
Copy Markdown
Member

@saimn , not sure how much you care about git credit but the last 3 commits not registered officially to your GitHub account. Do you want to fix that before merge?

pllim
pllim approved these changes Apr 22, 2022
View reviewed changes

@pllim pllim left a comment •
edited
Loading

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.

Just have one last question about git commit credit (see above), but otherwise this is good to go. Thanks!

@saimn

saimn commented Apr 22, 2022

Copy link
Copy Markdown
Contributor Author

@pllim - No it's fine, I guess I just need to add this email to my Github account (but thanks for noticing :)).

@pllim
pllim merged commit 212985e into astropy:main Apr 22, 2022
@saimn
saimn deleted the intensive-tests branch April 22, 2022 14:03
@pllim pllim mentioned this pull request Jul 4, 2022
Merged
10 tasks done
@pllim pllim mentioned this pull request Aug 19, 2023
Closed
@pllim pllim mentioned this pull request Dec 23, 2023
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@pllim pllim pllim approved these changes
@astrofrog astrofrog Awaiting requested review from astrofrog

Assignees

No one assigned

Labels

Docs Extra CI Run cron CI as part of PR installation io.ascii no-changelog-entry-needed Release testing units utils

Projects

None yet

Milestone

v5.1

Development

Successfully merging this pull request may close these issues.

3 participants

@saimn @pllim @nstarman