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

Post E2E and Apex test results to GitHub Checks #6075

Merged
merged 14 commits into from
Oct 10, 2024

Conversation

zivkan
Copy link
Member

@zivkan zivkan commented Oct 2, 2024

Bug

Fixes: https://github.com/NuGet/Client.Engineering/issues/2949

Description

  • Fix daily pipeline's trigger, and also remove the github checks name, since it doesn't need to report to github checks
  • Make apex and e2e templates take an optional github checks name, and when it's non-empty, report the test status
  • Make the build template take a list of checks, and for each one, set the github check to pending
  • Since NuGet.Client no longer has a single pipeline, there isn't a single list of check names that we'll want to report. It's easier to remove the validation set from the powershell script, than to keep maintaining it.

PR Checklist

  • Meaningful title, helpful description and a linked NuGet/Home issue
  • Added tests
  • Link to an issue or pull request to update docs if this PR changes settings, environment variables, new feature, etc.

@zivkan zivkan requested a review from a team as a code owner October 2, 2024 07:06
Copy link
Contributor

@jeffkl jeffkl left a comment

Choose a reason for hiding this comment

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

Somethings wrong the the Apex stage posting its status: https://devdiv.visualstudio.com/DevDiv/_build/results?buildId=10317783&view=logs&j=35fd8e9c-c657-57a4-085d-6f9cd010d4b2&t=98636fdc-df00-5b21-8d84-bd3fbb6c1630

I worry that since the end-to-end tests and Apex constantly fail, that it makes our PRs look like they're not ready for merge.

image

Are these pipelines going to become more stable soon and we don't need to worry about that?

@zivkan
Copy link
Member Author

zivkan commented Oct 2, 2024

I worry that since the end-to-end tests and Apex constantly fail, that it makes our PRs look like they're not ready for merge.
Are these pipelines going to become more stable soon and we don't need to worry about that?

That's up to the team. The tests were all passing before we had to switch to 1ES templates a few months ago, we just haven't been monitoring the tests since then. We might have introduced subtle regressions, or maybe it's just test bugs (for example, the underlying VS platform might have changed in ways that our tests can't handle). The team can choose to skip all the failing tests, to quickly get a green pipeline, taking the risk that if we have product bugs, we're not investigating it more quickly.

I don't understand why the VSDaily pipeline reported the overall pipeline status, but the non-daily pipeline did not. If it's configurable, I'd want it to be the other way around, but I would have thought it's not configurable, so either both should report, or neither would.

@nkolev92
Copy link
Member

nkolev92 commented Oct 7, 2024

I'm ok with having the checks posted, mostly because it saves me time looking for them through the pipelines :D
IMO without the checks I feel like it might be a bit of a out of sight, out of mind situation. I'm guessing we'll get them green faster if the checks are visible.

@zivkan
Copy link
Member Author

zivkan commented Oct 10, 2024

Well, there's only 2 people who commented here, one seemingly to merge as is (to raise awareness of the failing tests), another to skip the failing tests. Since the PR has been open for over a week, I'll cast my vote for raising awareness of failing tests, and merge as is. If the team finds it annoying enough, it's not too hard to skip them in another PR.

@zivkan zivkan merged commit 7c46398 into dev Oct 10, 2024
29 of 31 checks passed
@zivkan zivkan deleted the dev-zivkan-vs-tests-github-checks branch October 10, 2024 06:28
@zivkan zivkan added the Engineering Changes related to the build infrastructure of the repo and that do not change product code label Oct 16, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Engineering Changes related to the build infrastructure of the repo and that do not change product code
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants