Skip to content

Disable tests that fail on non-master branches #45455

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
tvolkert merged 1 commit into from
Nov 25, 2019
Merged

Disable tests that fail on non-master branches #45455

tvolkert merged 1 commit into from
Nov 25, 2019

Conversation

@tvolkert
Copy link
Contributor

@tvolkert tvolkert commented Nov 23, 2019

Description

Failing tests in non-master branches make us blind to real failures. This disables the ones that are currently failing (some because we've explicitly turned functionality off in non-master channels - others due to issues).

Related Issues

#45453

Checklist

  • I read the [Contributor Guide] and followed the process outlined there for submitting PRs.
  • I signed the [CLA].
  • I read and followed the [Flutter Style Guide], including [Features we expect every widget to implement].
  • I updated/added relevant documentation (doc comments with ///).
  • All existing and new tests are passing.
  • The analyzer (flutter analyze --flutter-repo) does not report any problems on my PR.
  • I am willing to follow-up on review comments in a timely manner.

@fluttergithubbot
Copy link
Contributor

fluttergithubbot commented Nov 23, 2019

It looks like this pull request may not have tests. Please make sure to add tests before merging. If you need an exemption to this rule, contact Hixie.

Reviewers: Read the Tree Hygiene page and make sure this patch meets those guidelines before LGTMing.

@fluttergithubbot fluttergithubbot added the c: contributor-productivity Team-specific productivity, code health, technical debt. label Nov 23, 2019
@tvolkert tvolkert requested a review from Hixie November 23, 2019 07:59
@tvolkert
Copy link
Contributor Author

tvolkert commented Nov 23, 2019

@fkorotkov can you review the changes to the Cirrus config file?

@xster
Copy link
Member

xster commented Nov 23, 2019

Is this a temporary disable for #45453? If so, do we just need to do it for the failing shard?

@xster
Copy link
Member

xster commented Nov 23, 2019

LGTM

@fkorotkov
Copy link
Contributor

fkorotkov commented Nov 23, 2019

Agreed with @xster about just disabling the failing shard. 🤔

@tvolkert
Copy link
Contributor Author

tvolkert commented Nov 24, 2019

do we just need to do it for the failing shard?

But it passes on PRs and on master. I'd prefer not to regress by having no test coverage at ToT.

@tvolkert
Copy link
Contributor Author

tvolkert commented Nov 24, 2019

@fkorotkov is the syntax correct (i.e. are grouping parenthesis supported in .cirrus.yml)?

@fkorotkov
Copy link
Contributor

fkorotkov commented Nov 24, 2019

Syntax is correct! () are supported. 👌

@xster
Copy link
Member

xster commented Nov 24, 2019

<just nitting/feel free to ignore> I don't mean to disable them across the board. I just meant to add the master filter like you're doing on fewer shards</>

@tvolkert
Copy link
Contributor Author

tvolkert commented Nov 25, 2019

Gonna merge as-is since I'm afk for the next 24 hours 🙂

@tvolkert tvolkert merged commit bae92c3 into flutter:master Nov 25, 2019
@tvolkert tvolkert deleted the devbeta branch December 3, 2019 19:10
@tvolkert
Copy link
Contributor Author

tvolkert commented Dec 3, 2019

@fkorotkov how come the tests are still running here: https://github.com/flutter/flutter/runs/331679315

@fkorotkov
Copy link
Contributor

fkorotkov commented Dec 3, 2019

@tvolkert I think it was triggered because of the chnagesInclude part for Windows tasks: https://github.com/flutter/flutter/blob/beta/.cirrus.yml#L406

I see that Cirrus found 300 files changed for the branch. 🤔

@tvolkert
Copy link
Contributor Author

tvolkert commented Dec 3, 2019

Ah, I see - thanks! I'll have to adjust the condition 🙂

@tvolkert tvolkert mentioned this pull request Dec 3, 2019
Merged
8 tasks
tvolkert added a commit that referenced this pull request Dec 4, 2019
@tvolkert
Revert "Disable tests that fail on non-master branches from running o…
de1b453 
…n those branches (#45455)"

This reverts commit bae92c3.
@tvolkert tvolkert mentioned this pull request Dec 4, 2019
Closed
tvolkert added a commit that referenced this pull request Dec 4, 2019
@tvolkert
Re-enable hostonly tests on non-master branches (#46037)
4c95bbd 
This is a follow-on to #45455 - the test failures indicated a
real problem, so re-enabling the tests on non-master.

#45453
@jmagman jmagman mentioned this pull request Feb 20, 2021
Merged
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Aug 2, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@Hixie Hixie Awaiting requested review from Hixie

Assignees

No one assigned

Labels

c: contributor-productivity Team-specific productivity, code health, technical debt.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@tvolkert @fluttergithubbot @xster @fkorotkov @googlebot