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

Run system tests on rennovate PR's proposing major version updates #1763

Closed
grayside opened this issue Apr 29, 2020 · 11 comments
Closed

Run system tests on rennovate PR's proposing major version updates #1763

grayside opened this issue Apr 29, 2020 · 11 comments
Assignees
Labels
priority: p2 Moderately-important priority. Fix may not be included in next release. sample samples Issues that are directly related to samples. type: cleanup An internal cleanup or hygiene concern.

Comments

@grayside
Copy link
Collaborator

Issue #1759 and the cluster addressed by PR #1680 were the result of merging a dependency update carrying a major version bump, which proceeded to break the system tests on run/logging-manual.

Would it be possible to run system tests on major version updates? This is creating escalated P1 bug work instead of P2 internal cleanup work.

@grayside grayside added type: cleanup An internal cleanup or hygiene concern. priority: p2 Moderately-important priority. Fix may not be included in next release. labels Apr 29, 2020
@ace-n
Copy link
Contributor

ace-n commented Apr 29, 2020

N.B: renovate PRs can typically have higher latency than human-written ones (in part because they're rarely, if ever, urgent) - so the attendant slowdown due to system tests is probably more palatable even if we run system tests on all renovate PRs.

@fhinkel
Copy link
Contributor

fhinkel commented Apr 30, 2020

Yes, we should run all tests on presubmit, just need to enablet those...
Anybody have spare cycles to add those test configs?

@ace-n
Copy link
Contributor

ace-n commented May 1, 2020

To be clear - @fhinkel you're saying we should run system tests on all PRs, or only all renovate ones?

(IIRC we may have disabled "system tests on all PRs" in the past because they were too slow. My argument is that that's less important for renovate PRs than human-generated ones.)

@grayside
Copy link
Collaborator Author

grayside commented May 1, 2020

Python is taking the approach of running Cloud Run system tests on every PR that touches it. Cloud Run build & deploy is generally 1-2 minutes overhead.

@fhinkel
Copy link
Contributor

fhinkel commented May 26, 2020

I'm in favor of running system tests on all PRs. But we have severa failing P1 system tests, so I can't turn them on on PRs. cc @JustinBeckwith

@fhinkel
Copy link
Contributor

fhinkel commented May 26, 2020

(IIRC we may have disabled "system tests on all PRs" in the past because they were too slow.

I don't remember running systems tests on PRs since moving to Kokoro.

@JustinBeckwith
Copy link
Contributor

With nodejs client libraries we run system tests on presubmit. If it's reasonable to do (I understand it isn't always), that would be my preference. If we have failing system tests, shouldn't those be getting picked up by build-cop-bot? I'm far more concerned that we have failing tests.

@grayside
Copy link
Collaborator Author

grayside commented Jul 6, 2020

Do we have a decision on how to move forward here?

@fhinkel
Copy link
Contributor

fhinkel commented Jul 24, 2020

Oh, I see we don't have open issues about system tests, now would be a good time to turn them on for PRs. Let me work on that!

@product-auto-label product-auto-label bot added sample samples Issues that are directly related to samples. labels Aug 19, 2020
@fhinkel
Copy link
Contributor

fhinkel commented Dec 9, 2020

For run, we always run the system-tests, whether pre-submit or nightly release, see

npm run --if-present system-test

The difference between release and presubmit are the appengine with deployment tests. We decided to not turn them on for presubmit as they take time.

For all other projects with should run system-tests on CI. But I suspect a lot of them are broken by now/never worked in the first place.

@fhinkel
Copy link
Contributor

fhinkel commented Dec 23, 2020

Fixed by #2118

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
priority: p2 Moderately-important priority. Fix may not be included in next release. sample samples Issues that are directly related to samples. type: cleanup An internal cleanup or hygiene concern.
Projects
None yet
Development

No branches or pull requests

4 participants