Skip to content
This repository was archived by the owner on Feb 25, 2025. It is now read-only.

Skip the skwasm unit test suite on Safari since it is flaky. #37602

Merged
merged 2 commits into from
Nov 15, 2022
Merged

Skip the skwasm unit test suite on Safari since it is flaky. #37602

merged 2 commits into from
Nov 15, 2022

Conversation

eyebrowsoffire
Copy link
Contributor

We're having some periodic flakes of this part of the test suite on the Safari webdriver only. I suspect it has to do with only having a single unit test in the suite, but I'm not positive. Let's just disable this for now, I filed a follow-on issue for deeper investigation later: flutter/flutter#115312

@flutter-dashboard
Copy link

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 on the #hackers channel in Chat (don't just cc him here, he won't see it! He's on Discord!).

If you are not sure if you need tests, consider this rule of thumb: the purpose of a test is to make sure someone doesn't accidentally revert the fix. Ask yourself, is there anything in your PR that you feel it is important we not accidentally revert back to how it was before your fix?

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

@flutter-dashboard flutter-dashboard bot added platform-web Code specifically for the web engine needs tests labels Nov 14, 2022
@Hixie
Copy link
Contributor

Hixie commented Nov 14, 2022

test-exempt: is a test

yjbanov
yjbanov reviewed Nov 15, 2022
View reviewed changes
lib/web_ui/dev/steps/run_tests_step.dart
@@ -100,7 +100,9 @@ class RunTestsStep implements PipelineStep {
);
}

if (sortedTests.skwasmTests.isNotEmpty) {
// For some reason, Safari is flaky when running the Skwasm test suite
// See https://github.com/flutter/flutter/issues/115312
Copy link
Contributor

Choose a reason for hiding this comment

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

Let's make it a // TODO so it is tracked by the tech debt tracker.

mdebbar
mdebbar approved these changes Nov 15, 2022
View reviewed changes
Copy link
Contributor

@mdebbar mdebbar left a comment

Choose a reason for hiding this comment

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

LGTM after Yegor's suggestion to make it a TODO.

@eyebrowsoffire eyebrowsoffire added the autosubmit Merge PR when tree becomes green via auto submit App label Nov 15, 2022
@auto-submit auto-submit bot merged commit 3a622e1 into flutter:main Nov 15, 2022
@engine-flutter-autoroll engine-flutter-autoroll mentioned this pull request Nov 15, 2022
Merged
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Nov 15, 2022
@engine-flutter-autoroll
3a622e17a Skip the skwasm unit test suite on Safari since it is flaky. (
e36ddc6 
flutter/engine#37602)
auto-submit bot pushed a commit to flutter/flutter that referenced this pull request Nov 15, 2022
@engine-flutter-autoroll
Roll Flutter Engine from 84cb1f833ac4 to af6e47f1d5c4 (4 revisions) (#…
792eefe 
…115395)

* 3a622e17a Skip the skwasm unit test suite on Safari since it is flaky. (flutter/engine#37602)

* f1206f265 [web] Test harness fixes. (flutter/engine#37560)

* c628e1f26 Roll Fuchsia Mac SDK from 32kfSm94FGtJJeUjg... to gKyvwhUS3r6pTRfxx... (flutter/engine#37634)

* af6e47f1d Removing some builds from mac release builder. (flutter/engine#37556)
@eyebrowsoffire eyebrowsoffire mentioned this pull request Nov 16, 2022
Closed
schwa423 pushed a commit to schwa423/engine that referenced this pull request Nov 16, 2022
@eyebrowsoffire @schwa423
Skip the skwasm unit test suite on Safari since it is flaky. (flutter…
7da4ef1 
…#37602)

* Skip the skwasm unit test suite on Safari since it is flaky.

* Add TODO.
godofredoc added a commit that referenced this pull request Nov 16, 2022
@godofredoc @gaaclarke @eyebrowsoffire
Clang tidy and web engine cherrypicks (#37660)
c48ca60 
* clang-tidy: added the ability to shard jobs (#37265)

* clang-tidy: added the ability to shard jobs

* added test

* jenn feedback

* hack ci to run as a shard to measure the time

* tweak

* fix hack

* zach feedback

* zach feedback 2

* removed stray async

* moved to using sets for lookups

* fixed typo in docstring

* Revert "fix hack"

This reverts commit 06a61a6.

Revert "tweak"

This reverts commit e7c58b1.

Revert "hack ci to run as a shard to measure the time"

This reverts commit e458963.

* removed calls to map

* turned the ci hack back on

* Revert "turned the ci hack back on"

This reverts commit 0d53794.

* removed sync*

* Clang-tidy: Fixed math on shard-id validator. (#37433)

Clang-tidy: Fixed math on shard-id validator.

* Felt analyze (#37481)

* Adding `felt analyze` command that CI will run.

* Remove some copypasta'd stuff.

* Also remove code path from felt.dart that forces a rebuild if it doesn't
detect the host_debug_unopt directory.

* More cleanup of felt.bat for CI.

* Fix typo in felt.bat.

* Run pub get before building host.dart. (#37502)

* Run pub get before building host.dart.

* We should call `pub get` for `web_ui` in the launcher script because
felt itself needs it. However, we should let felt invoke `pub get` on
`web_engine_tester` only as needed, not in the launcher script.

* Skip the skwasm unit test suite on Safari since it is flaky. (#37602)

* Skip the skwasm unit test suite on Safari since it is flaky.

* Add TODO.

* Remove felt snapshotting behavior. (#37639)

* Remove felt snapshotting behavior.

* Use `dart run`.

* Combine results of all the test batches. (#37610)

* Combine results of all the test batches.

* Skip regressions

* Use bool instead

* remove unused var

* skip fragment_program_test

* Also skip GL context lost test

* Transparent background test fails on Firefox and Safari

* Skip other test in safari

* Skip text test on firefox

Co-authored-by: gaaclarke <[email protected]>
Co-authored-by: Jackson Gardner <[email protected]>
Co-authored-by: Harry Terkelsen <[email protected]>
shogohida pushed a commit to shogohida/flutter that referenced this pull request Dec 7, 2022
@engine-flutter-autoroll @shogohida
Roll Flutter Engine from 84cb1f833ac4 to af6e47f1d5c4 (4 revisions) (f…
1a1b260 
…lutter#115395)

* 3a622e17a Skip the skwasm unit test suite on Safari since it is flaky. (flutter/engine#37602)

* f1206f265 [web] Test harness fixes. (flutter/engine#37560)

* c628e1f26 Roll Fuchsia Mac SDK from 32kfSm94FGtJJeUjg... to gKyvwhUS3r6pTRfxx... (flutter/engine#37634)

* af6e47f1d Removing some builds from mac release builder. (flutter/engine#37556)
gspencergoog pushed a commit to gspencergoog/flutter that referenced this pull request Jan 19, 2023
@engine-flutter-autoroll @gspencergoog
Roll Flutter Engine from 84cb1f833ac4 to af6e47f1d5c4 (4 revisions) (f…
b83bf27 
…lutter#115395)

* 3a622e17a Skip the skwasm unit test suite on Safari since it is flaky. (flutter/engine#37602)

* f1206f265 [web] Test harness fixes. (flutter/engine#37560)

* c628e1f26 Roll Fuchsia Mac SDK from 32kfSm94FGtJJeUjg... to gKyvwhUS3r6pTRfxx... (flutter/engine#37634)

* af6e47f1d Removing some builds from mac release builder. (flutter/engine#37556)
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@yjbanov yjbanov yjbanov approved these changes

@mdebbar mdebbar mdebbar approved these changes

@godofredoc godofredoc godofredoc approved these changes

Assignees
No one assigned
Labels
autosubmit Merge PR when tree becomes green via auto submit App needs tests platform-web Code specifically for the web engine
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@eyebrowsoffire @Hixie @yjbanov @mdebbar @godofredoc