Skip to content
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.

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

Skip DB tests duplicate runs on push to branches #23476

Merged
merged 5 commits into from
Mar 15, 2023

Conversation

techknowlogick
Copy link
Member

@techknowlogick techknowlogick commented Mar 14, 2023

This skips all testing-* pipelines on push to main or release/* branches. This decreases the total build time on those, as in theory they should already be run for PRs before merging.

Fixes #22011

@techknowlogick techknowlogick added type/testing skip-changelog This PR is irrelevant for the (next) changelog, for example bug fixes for unreleased features. backport/manual No power to the bots! Create your backport yourself! labels Mar 14, 2023
@techknowlogick techknowlogick added this to the 1.20.0 milestone Mar 14, 2023
@jolheiser
Copy link
Member

Ah thanks, I was also looking at something for this.

One thing, we may need the mysql pipeline to run still, as that includes our coverage report.

@GiteaBot GiteaBot added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label Mar 14, 2023
@jolheiser
Copy link
Member

This also partially covers #22011, though this doesn't cover tags afaict

@techknowlogick
Copy link
Member Author

This also partially covers #22011, though this doesn't cover tags afaict

just update to skip tags.

One thing, we may need the mysql pipeline to run still, as that includes our coverage report.

I think we need to reflect on our coverage reports, and if they are in fact used/reviewed.

@jolheiser
Copy link
Member

jolheiser commented Mar 14, 2023

With your latest changes, is it simpler to just remove push from the events rather than exclude?
That was my approach in my branch.

Further, some of the release pipelines depend on these test pipelines, if they don't run does that need to change? I believe I just changed them to depend on compliance instead. (Hadn't checked if that's correct, though)

@techknowlogick
Copy link
Member Author

With your latest changes, is it simpler to just remove push from the events rather than exclude? That was my approach in my branch.

Yes, it certainly would (you can see it in the testing-e2e pipeline), I went for an exclude if for some reason we need to push a branch that isn't main or release/* and want to run things. Definitely not the cleanest approach, and possibly am attempting to optimizing for something that may never happen. Happy to change if you (or others) have strong feelings about it.

Further, some of the release pipelines depend on these test pipelines, if they don't run does that need to change? I believe I just changed them to depend on compliance instead. (Hadn't checked if that's correct, though)

So the fun (in a good way) thing is that drone will adjust the dag to accommodate for skipped pipelines (see: https://gitea.com/gitea/drone-test/src/branch/main/.drone.yml for an example of how this plays out in practice)

@techknowlogick
Copy link
Member Author

LOLOL, yup. This doesn't work. I'll need to change it to the way you suggested @jolheiser

@jolheiser
Copy link
Member

I do think I prefer changing to removing push instead, only because it will be clearer in a year when I look again and think "what is this doing???" 😂

Just my opinion, though, I'm fine with approving either once we get it passing. 🙂

@techknowlogick
Copy link
Member Author

techknowlogick commented Mar 14, 2023

@jolheiser my "LLOLOL" comment above was checking out the run, and seeing that drone uses the "branch" condition in the evaluation for PRs, so if I say "skip main" then it'll skip that pipeline for PRs attempting to merge into main. So the way you suggested is likely the only way to go. I've updated to just remove push

@GiteaBot GiteaBot added lgtm/need 1 This PR needs approval from one additional maintainer to be merged. and removed lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. labels Mar 14, 2023
@GiteaBot GiteaBot added lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. and removed lgtm/need 1 This PR needs approval from one additional maintainer to be merged. labels Mar 15, 2023
@lunny lunny added the reviewed/wait-merge This pull request is part of the merge queue. It will be merged soon. label Mar 15, 2023
@techknowlogick techknowlogick merged commit f96eef8 into go-gitea:main Mar 15, 2023
@techknowlogick techknowlogick deleted the skip-tests-on-branches branch March 15, 2023 01:46
@lunny lunny removed the reviewed/wait-merge This pull request is part of the merge queue. It will be merged soon. label Mar 15, 2023
zjjhot added a commit to zjjhot/gitea that referenced this pull request Mar 15, 2023
* giteaofficial/main: (33 commits)
  Bump webpack from 5.75.0 to 5.76.0 (go-gitea#23484)
  Replace Less with CSS (go-gitea#23481)
  Fix 'View File' button in code search (go-gitea#23478)
  Use `gitea/test_env` image instead of `golang` (go-gitea#23455)
  Skip DB tests duplicate runs on push to branches (go-gitea#23476)
  Update app.example.ini (go-gitea#23480)
  [skip ci] Updated translations via Crowdin
  Fix due date being wrong on issue list (go-gitea#23475)
  test_env: hardcode major go version in use (go-gitea#23464)
  Push option bonus for PTC docs (go-gitea#23473)
  Lint Markdown pass
  Push to create docs (go-gitea#23458)
  Convert GitHub event on actions and fix some pull_request events. (go-gitea#23037)
  Remove wrongly added column on migration test fixtures (go-gitea#23456)
  Refactor branch/tag selector to Vue SFC (go-gitea#23421)
  add admin API email endpoints (go-gitea#22792)
  add user rename endpoint to admin api (go-gitea#22789)
  Add workflow error notification in ui (go-gitea#23404)
  Make branches list page operations remember current page (go-gitea#23420)
  fix markdown lint issue (go-gitea#23457)
  ...
@lunny lunny added the outdated/backport/v1.19 This PR should be backported to Gitea 1.19 label Mar 26, 2023
@go-gitea go-gitea locked and limited conversation to collaborators May 3, 2023
@lunny lunny removed the outdated/backport/v1.19 This PR should be backported to Gitea 1.19 label Jul 26, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
backport/manual No power to the bots! Create your backport yourself! lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. skip-changelog This PR is irrelevant for the (next) changelog, for example bug fixes for unreleased features. type/testing
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Eliminate CI duplication
4 participants