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.

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

Use search API to find matching PR for ci.yaml validation reruns #2402

Merged
merged 10 commits into from
Jan 20, 2023
Merged

Use search API to find matching PR for ci.yaml validation reruns #2402

merged 10 commits into from
Jan 20, 2023

Conversation

nehalvpatel
Copy link
Contributor

According to flutter/flutter#100081, this should fix ci.yaml validation errors.

Pre-launch Checklist

  • I read the [Contributor Guide] and followed the process outlined there for submitting PRs.
  • I read the [Tree Hygiene] wiki page, which explains my responsibilities.
  • I read the [Flutter Style Guide] recently, and have followed its advice.
  • I signed the [CLA].
  • I listed at least one issue that this PR fixes in the description above.
  • I updated/added relevant documentation (doc comments with ///).
  • I added new tests to check the change I am making, or this PR is [test-exempt].
  • All existing and new tests are passing.

godofredoc
godofredoc reviewed Jan 18, 2023
View reviewed changes
app_dart/lib/src/service/github_service.dart Outdated Show resolved Hide resolved
app_dart/lib/src/service/github_service.dart Outdated Show resolved Hide resolved
app_dart/lib/src/service/scheduler.dart Outdated Show resolved Hide resolved
app_dart/lib/src/foundation/github_checks_util.dart Outdated
@@ -40,6 +40,27 @@ class GithubChecksUtil {
);
}

/// Finds all check suites that are associated with a given git [ref].
Future<Stream<github.CheckSuite>> listCheckSuitesForRef(
Copy link
Contributor

Choose a reason for hiding this comment

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

Tests for this method?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Can you suggest how I should test this? I added a test for searchIssuesAndPRs to validate that it encodes the query properly, but for listCheckSuitesForRef and getPullRequest I can't think of anything since they just forward the call unconditionally.

app_dart/lib/src/service/github_service.dart Outdated Show resolved Hide resolved
app_dart/lib/src/service/scheduler.dart Outdated Show resolved Hide resolved
@nehalvpatel
Copy link
Contributor Author

Setting to draft while I fix the scheduler test.

@nehalvpatel nehalvpatel marked this pull request as draft January 19, 2023 19:36
@nehalvpatel nehalvpatel marked this pull request as ready for review January 19, 2023 22:50
godofredoc
godofredoc approved these changes Jan 19, 2023
View reviewed changes
Copy link
Contributor

@godofredoc godofredoc left a comment

Choose a reason for hiding this comment

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

Can we manually release a version to test this change?

@nehalvpatel
Copy link
Contributor Author

Can we manually release a version to test this change?

I was able to deploy it and successfully perform a ci.yaml validation rerun on flutter/flutter#118690. 😄

The only hitch is that previously failed webhook events which requested to rerun ci.yaml keep being retried. The problem is that the PRs associated are already closed or have had a new commit, which invalidates the check suite. This causes the event processing to fail and is causing looping retries. I think we might have to drain those events.

@godofredoc
Copy link
Contributor

I think we might have to drain those events.

You can take a look at the subscription, it may already be configured to auto drop messages older than 24h

@nehalvpatel
Copy link
Contributor Author

Looks like it is set to 7 days.

Screenshot 2023-01-20 at 11 32 24 AM

@nehalvpatel nehalvpatel mentioned this pull request Jan 20, 2023
Merged
@nehalvpatel nehalvpatel added the autosubmit Merge PR when tree becomes green via auto submit App. label Jan 20, 2023
@auto-submit auto-submit bot merged commit 07613fc into flutter:main Jan 20, 2023
@nehalvpatel nehalvpatel deleted the ci_yaml_rerun branch January 21, 2023 00:37
This was referenced Jan 21, 2023
Closed
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@CaseyHillers CaseyHillers CaseyHillers left review comments

@godofredoc godofredoc godofredoc approved these changes

Assignees
No one assigned
Labels
autosubmit Merge PR when tree becomes green via auto submit App.
Projects
None yet
Milestone
No milestone
3 participants
@nehalvpatel @godofredoc @CaseyHillers