Skip to content

TST: Skip test_ignore_sigint in predeps job#15818

Merged
pllim merged 3 commits intoastropy:mainfrom
pllim:ignore-ignore-sigint
Jan 9, 2024
Merged

TST: Skip test_ignore_sigint in predeps job#15818
pllim merged 3 commits intoastropy:mainfrom
pllim:ignore-ignore-sigint

Conversation

@pllim
Copy link
Copy Markdown
Member

@pllim pllim commented Jan 6, 2024

Description

Because I do not know how to fix it for all the jobs all at once. See #15817 for failed attempt.

This is a direct follow-up of #15809 to fix #15807 . One way we can fix #15827 .

  • By checking this box, the PR author has requested that maintainers do NOT use the "Squash and Merge" button. Maintainers should respect this when possible; however, the final decision is at the discretion of the maintainer that merges the PR.

@pllim pllim added this to the v6.0.1 milestone Jan 6, 2024
@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Jan 6, 2024

Thank you for your contribution to Astropy! 🌌 This checklist is meant to remind the package maintainers who will review this pull request of some common things to look for.

  • 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 instructions for rebase and squash.
  • 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. Codestyle issues can be fixed by the bot.
  • 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 this a big PR that makes a "What's new?" entry worthwhile and if so, is (1) a "what's new" entry included in this PR and (2) the "whatsnew-needed" label applied?
  • Is a milestone set? Milestone must be set but we cannot check for it on Actions; 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.

@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Jan 6, 2024

👋 Thank you for your draft pull request! Do you know that you can use [ci skip] or [skip ci] in your commit messages to skip running continuous integration tests until you are ready?

@pllim

This comment was marked as resolved.

pllim added 2 commits January 9, 2024 12:19
because I do not know how to fix it for all the jobs all at once.
See PR 15817 for failed attempt.
instead of skipping it blindly
@pllim pllim force-pushed the ignore-ignore-sigint branch from 2a04b61 to 8059e1b Compare January 9, 2024 17:25
@pllim
Copy link
Copy Markdown
Member Author

pllim commented Jan 9, 2024

@saimn , now it is skipping based on thread count as you suggested but the skip check has to happen at run time, not collection time. Does this look good to you? It does fix the predeps job. Other failures are unrelated (exotic archs still running when I typed this).

@pllim
Copy link
Copy Markdown
Member Author

pllim commented Jan 9, 2024

I really want this failure to go away so I can test other pytest 8.0rc stuff. Merging. If you disagree, feel free to open follow-up PR. Thanks!

@pllim pllim merged commit 5ad5b66 into astropy:main Jan 9, 2024
@pllim pllim deleted the ignore-ignore-sigint branch January 9, 2024 21:26
meeseeksmachine pushed a commit to meeseeksmachine/astropy that referenced this pull request Jan 9, 2024
pllim added a commit that referenced this pull request Jan 9, 2024
…818-on-v6.0.x

Backport PR #15818 on branch v6.0.x (TST: Skip test_ignore_sigint in predeps job)
@saimn
Copy link
Copy Markdown
Contributor

saimn commented Jan 10, 2024

@pllim - Looks good to me, thanks! (Indeed checking the number of threads must be done when running the test, otherwise the other threads are not alive).

@pllim
Copy link
Copy Markdown
Member Author

pllim commented Jan 10, 2024

BTW @saimn , chanley said the fix was necessary 17 years ago for something but he didn't have the resources to implement it also for new buffer. So, I don't think we should remove it but I am also not sure if it is worth our time to implement it for new buffer. 🤷‍♀️

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

What to do with test_ignore_sigint Test failures in predeps job (pre-release dependencies)

2 participants