Skip to content

Remove the on_pull_request_target trigger from the download test. #3399

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 7 commits into from
Jan 29, 2025
Merged

Remove the on_pull_request_target trigger from the download test. #3399

merged 7 commits into from
Jan 29, 2025

Conversation

@MatthiasZepper
Copy link
Member

@MatthiasZepper MatthiasZepper commented Jan 15, 2025 •
edited
Loading

Admittedly, I had used GitHub Copilot to pick the triggers for the Download test workflow. However, it turns out that running on pull_request_target is useless, since the download test will only be run on the release PRs from dev to main. and a released pipeline should have passed that test before.

Additionally, I am removing the edited web hook, since editing the text of the PR description is not really a useful trigger for testing the download.

PR checklist

  • This comment contains a description of changes (with reason)
  • CHANGELOG.md is updated
  • If you've fixed a bug or added code that should be tested, add tests!
  • Documentation in docs is updated

@jfy133 jfy133 mentioned this pull request Jan 15, 2025
Closed
@MatthiasZepper MatthiasZepper added this to the 3.2 milestone Jan 16, 2025
@mashehu mashehu mentioned this pull request Jan 20, 2025
Merged
mirpedrol
mirpedrol reviewed Jan 21, 2025
View reviewed changes
Copy link
Member

@mirpedrol mirpedrol left a comment

Choose a reason for hiding this comment

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

LGTM

@mirpedrol mirpedrol modified the milestones: 3.2, 3.3.0 Jan 27, 2025
@MatthiasZepper MatthiasZepper force-pushed the download_action_trigger_refactor branch from b051b11 to 911b139 Compare January 27, 2025 17:41
@MatthiasZepper MatthiasZepper linked an issue Jan 27, 2025 that may be closed by this pull request
Too many redundant tests executed on dev to master release for pipelines #3400
Closed
mirpedrol
mirpedrol approved these changes Jan 28, 2025
View reviewed changes
Copy link
Member

@mirpedrol mirpedrol left a comment

Choose a reason for hiding this comment

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

LGTM, thanks!

@mirpedrol
Copy link
Member

I am looking at the failing test now 👀

@MatthiasZepper MatthiasZepper force-pushed the download_action_trigger_refactor branch from f933e58 to 72150cf Compare January 28, 2025 16:23
@MatthiasZepper MatthiasZepper force-pushed the download_action_trigger_refactor branch from 4d09d91 to c81b818 Compare January 28, 2025 20:19
@MatthiasZepper
Copy link
Member Author

I am looking at the failing test now 👀

I hope you fixed it, but I somehow can't launch a workflow run with the GH Action version from my branch in this repo.

This works, because it is my fork:

 gh workflow run -R "MatthiasZepper/tools" --ref "download_action_trigger_refactor" "pytest.yml"

but

 gh workflow run -R "nf-core/tools" --ref "refs/pull/3399/head" "pytest.yml"

doesn't.

Unfortunately, I have no clue what the correct ref would be. Neither refs/pull/3399, refs/heads/pull/3399, pull/3399/head:download_action_trigger_refactor, MatthiasZepper:download_action_trigger_refactor nor MatthiasZepper/tools:download_action_trigger_refactor seem to work.

@mirpedrol
Copy link
Member

Hi @MatthiasZepper, I just pushed the fix.
Why do you want to launch the action? tests are running on the PR CI

@codecov
Copy link

codecov bot commented Jan 29, 2025 •
edited
Loading

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 76.44%. Comparing base (0ede9d5) to head (09b9e2a).
Report is 21 commits behind head on dev.

Additional details and impacted files

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@MatthiasZepper MatthiasZepper merged commit f332e85 into nf-core:dev Jan 29, 2025
91 checks passed
@MatthiasZepper MatthiasZepper deleted the download_action_trigger_refactor branch January 29, 2025 15:08
@MatthiasZepper
Copy link
Member Author

Hi @MatthiasZepper, I just pushed the fix. Why do you want to launch the action? tests are running on the PR CI

I was confused about the failing test, because I recalled that you had before already modified the CI to adapt it to that change, and thus attributed that failure to an old version of the CI. To my best knowledge, it will always run the CI from main, which is why I wanted to launch a custom run with the workflow file from my branch to prove that it would pass there. But if you managed now like this, there is indeed no reason. Happy to merge!

@mashehu mashehu mentioned this pull request Feb 20, 2025
Merged
11 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@mirpedrol mirpedrol mirpedrol approved these changes

@mashehu mashehu Awaiting requested review from mashehu

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

3.3.0

Development

Successfully merging this pull request may close these issues.

Too many redundant tests executed on dev to master release for pipelines

4 participants

@MatthiasZepper @mirpedrol @mashehu @nf-core-bot