Skip to content

Reland "[web] Add the flutter-gold check in web engine PRs" #1546

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
fluttergithubbot merged 2 commits into from
Jan 14, 2022
Merged

Reland "[web] Add the flutter-gold check in web engine PRs" #1546

fluttergithubbot merged 2 commits into from
Jan 14, 2022

Conversation

@mdebbar
Copy link
Contributor

@mdebbar mdebbar commented Jan 12, 2022

Relanding #1505
Was reverted in #1541

The entire original PR is in the first commit. All subsequent commits are the additional changes in the relanding.

The original PR was reverted because the GraphQL query was broken:

OperationException(
  linkException: null,
  graphqlErrors: [
    GraphQLError(message: Variable $sRepoOwner is used by ChecksForPullRequest but not declared, locations: [ErrorLocation(line: 2, column: 21)], path: [query ChecksForPullRequest, repository, owner], extensions: {code: variableNotDefined, variableName: sRepoOwner}),
    GraphQLError(message: Variable $sRepoName is used by ChecksForPullRequest but not declared, locations: [ErrorLocation(line: 2, column: 40)], path: [query ChecksForPullRequest, repository, name], extensions: {code: variableNotDefined, variableName: sRepoName})
  ]
)

@mdebbar
Copy link
Contributor Author

mdebbar commented Jan 12, 2022

I couldn't find a way to write a test for the graphql error (any ideas?).

So I'm trying to run it locally or in staging to confirm that it works with no issues.

@mdebbar mdebbar mentioned this pull request Jan 12, 2022
Closed
6 tasks
@mdebbar
Copy link
Contributor Author

mdebbar commented Jan 12, 2022

@Piinks I deployed an instance of flutter-dashboard containing this PR. I sent a curl request to /api/push-gold-status-to-github to test.

The result was some spam messages on some old still-open PRs :P but it's working and the flutter-gold check is applied to flutter and engine PRs.

Example in flutter/engine: https://github.com/flutter/engine/pull/30816/checks.

@CaseyHillers
Copy link
Contributor

CaseyHillers commented Jan 12, 2022

I couldn't find a way to write a test for the graphql error (any ideas?).

\cc @godofredoc

You could write an integration test that does an example graphql run and tries to process the response. That would require us to enable LUCI to have access to /api/query-github-graphql.

@mdebbar mdebbar added the waiting for tree to go green Merge PR when tree becomes green via fluttergithubbot label Jan 14, 2022
@fluttergithubbot fluttergithubbot merged commit 481ad34 into flutter:main Jan 14, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@Piinks Piinks Awaiting requested review from Piinks Piinks is a code owner

@keyonghan keyonghan Awaiting requested review from keyonghan

1 more reviewer

@CaseyHillers CaseyHillers CaseyHillers approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

waiting for tree to go green Merge PR when tree becomes green via fluttergithubbot

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@mdebbar @CaseyHillers @fluttergithubbot