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

Re-land "Update Zulip tests and re-enable (#441)" #446

Merged
merged 2 commits into from
Feb 5, 2025

Conversation

gnprice
Copy link
Member

@gnprice gnprice commented Jan 23, 2025

This re-lands 8cf7a67 / #441.

Difference from previous version: This version has the new setup step do its work only where sudo is available (in GitHub Actions), and successfully do nothing where it's unavailable (in LUCI). The step isn't needed in LUCI.

The logic lives in a setup script added today to the Zulip tree:
zulip/zulip-flutter#1300

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.

If you need help, consider asking for advice on the #hackers-new channel on Discord.

This re-lands 8cf7a67.

Difference from previous version: This version has the new setup step
do its work only where `sudo` is available (in GitHub Actions), and
successfully do nothing where it's unavailable (in LUCI).  The step
isn't needed in LUCI.

The logic lives in a setup script added today to the Zulip tree:
  zulip/zulip-flutter#1300
@gnprice
Copy link
Member Author

gnprice commented Jan 23, 2025

Marked as draft because before we land this, I'd like to do a test run that confirms it does indeed work smoothly in LUCI. I'm fairly confident it will — but since the GitHub Actions / LUCI contrast already turned the tree red once this week, I'd rather make sure it won't again.

(Alternatively if flutter/flutter#162048 lands first, then this can go ahead and land and we'll get that confirmation when updating the flutter/tests pin in flutter/flutter.)

I'd initially entered the commit ID from my zulip/zulip-flutter PR,
rather than the commit ID resulting from the merge, oops.  So the
former wasn't present in the cloned repo.
@gnprice
Copy link
Member Author

gnprice commented Jan 24, 2025

OK, over in flutter/flutter#162120 I successfully ran this PR's version of customer_testing, and it passed:

+ git -C ../../bin/cache/pkg/tests checkout FETCH_HEAD
HEAD is now at f0d3df3 Use correct commit ID
+ dart --enable-asserts run_tests.dart --skip-on-fetch-failure --skip-template ../../bin/cache/pkg/tests/registry/assorted_layout_widgets.test ../../bin/cache/pkg/tests/registry/dart_rfb.test ../../bin/cache/pkg/tests/registry/flutter_cocoon.test ../../bin/cache/pkg/tests/registry/flutter_devtools_analysis.test ../../bin/cache/pkg/tests/registry/flutter_devtools.test ../../bin/cache/pkg/tests/registry/flutter_portal.test ../../bin/cache/pkg/tests/registry/flutter_rfb.test ../../bin/cache/pkg/tests/registry/macos_ui.test ../../bin/cache/pkg/tests/registry/provider.test ../../bin/cache/pkg/tests/registry/super_editor.test ../../bin/cache/pkg/tests/registry/super_sliver_list.test ../../bin/cache/pkg/tests/registry/template.test ../../bin/cache/pkg/tests/registry/zulip.test

Processing ./../../bin/cache/pkg/tests/registry/assorted_layout_widgets.test...
Tests finished in 18.00 seconds per iteration.
[…]
Processing ./../../bin/cache/pkg/tests/registry/super_editor.test...
Tests finished in 220.00 seconds per iteration.
Processing ./../../bin/cache/pkg/tests/registry/super_sliver_list.test...
Tests finished in 38.00 seconds per iteration.
Processing ./../../bin/cache/pkg/tests/registry/zulip.test...
Tests finished in 99.00 seconds per iteration.
19 tests all passed!
ELAPSED TIME: 34min 8.683s for ../../ci.sh  in /b/s/w/ir/x/w/flutter/dev/customer_testing
▌21:06:48▐ Test successful.

LUCI logs:
https://ci.chromium.org/ui/p/flutter/builders/try/Linux%20customer_testing/91700/overview
https://logs.chromium.org/logs/flutter/buildbucket/cr-buildbucket/8724890549871873921/+/u/run_test.dart_for_customer_testing_shard_and_subshard_None/stdout

Meanwhile it also passed here in the GitHub Actions-based CI for this repo. So I think everything looks clear for this to merge.

@gnprice gnprice marked this pull request as ready for review January 24, 2025 05:54
@gnprice gnprice requested review from matanlurey and Piinks February 5, 2025 01:41
Copy link
Contributor

@matanlurey matanlurey left a comment

Choose a reason for hiding this comment

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

Thanks for iterating!

@auto-submit auto-submit bot merged commit 6a6d096 into flutter:main Feb 5, 2025
12 checks passed
@gnprice gnprice deleted the pr-zulip branch February 5, 2025 19:20
gnprice added a commit to gnprice/flutter that referenced this pull request Feb 5, 2025
Looks like this is the first roll since this pin was introduced
in flutter#162041 / flutter#162048.

For fun and the possible convenience of the next person doing one of
these, here's the command I used to generate the log below:

    $ git log --oneline --no-decorate --format='%cs %aE %s' b4cc09721.. \
        | perl -pe 's,#,flutter/tests#,g'

flutter/tests@b4cc097...6a6d096

2025-02-05 [email protected] Re-land "Update Zulip tests and re-enable (flutter/tests#441)" (flutter/tests#446)
2025-01-30 [email protected] Update contact for pkg:animations (flutter/tests#447)
2025-01-24 [email protected] Add a PR template that explains updating `tests.version` (flutter/tests#445)
github-merge-queue bot pushed a commit to flutter/flutter that referenced this pull request Feb 5, 2025
Looks like this is the first roll since this pin was introduced in
#162041 / #162048.

For fun and the possible convenience of the next person doing one of
these, here's the command I used to generate the log below:

$ git log --oneline --no-decorate --format='%cs %aE %s' b4cc09721.. \
        | perl -pe 's,#,flutter/tests#,g'

flutter/tests@b4cc097...6a6d096

2025-02-05 [email protected] Re-land "Update Zulip tests and re-enable
(flutter/tests#441)" (flutter/tests#446)
2025-01-30 [email protected] Update contact for pkg:animations
(flutter/tests#447)
2025-01-24 [email protected] Add a PR template that
explains updating `tests.version` (flutter/tests#445)

## Pre-launch Checklist

- [x] I read the [Contributor Guide] and followed the process outlined
there for submitting PRs.
- [x] I read the [Tree Hygiene] wiki page, which explains my
responsibilities.
- [x] I read and followed the [Flutter Style Guide], including [Features
we expect every widget to implement].
- [x] 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 `///`).
- [x] I added new tests to check the change I am making, or this PR is
[test-exempt].
- [x] I followed the [breaking change policy] and added [Data Driven
Fixes] where supported.
- [x] All existing and new tests are passing.

If you need help, consider asking for advice on the #hackers-new channel
on [Discord].

<!-- Links -->
[Contributor Guide]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#overview
[Tree Hygiene]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md
[test-exempt]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#tests
[Flutter Style Guide]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Style-guide-for-Flutter-repo.md
[Features we expect every widget to implement]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Style-guide-for-Flutter-repo.md#features-we-expect-every-widget-to-implement
[CLA]: https://cla.developers.google.com/
[flutter/tests]: https://github.com/flutter/tests
[breaking change policy]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#handling-breaking-changes
[Discord]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Chat.md
[Data Driven Fixes]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Data-driven-Fixes.md
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants