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

[web] Running integration tests on Safari on Local #18488

Merged
merged 5 commits into from
Jun 18, 2020
Merged

[web] Running integration tests on Safari on Local #18488

merged 5 commits into from
Jun 18, 2020

Conversation

nturgut
Copy link
Contributor

@nturgut nturgut commented May 18, 2020 •
edited
Loading

Changes to felt to be able to run integration tests on Safari both on LUCI and on local MacOs machines.

fixes: flutter/flutter#57348

currently blocked on: flutter/flutter#57758

yjbanov
yjbanov reviewed May 26, 2020
View reviewed changes
lib/web_ui/dev/integration_tests_manager.dart Outdated
];
}

String getStatementToRun(String testName, String mode) {
Copy link
Contributor

Choose a reason for hiding this comment

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

This should be just => 'flutter ${getTestArguments(testName, mode).join(' ')}'

Copy link
Contributor Author

Choose a reason for hiding this comment

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

But it differs depending on where it runs. Check next lines. There is an if statement for LUCI. Trying to squeeze this method like that will make it less readable.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

changed the one in the other class (SafariIntegrationArguments).

@nturgut nturgut force-pushed the safari_int_on_luci branch from b227b52 to 0a8a320 Compare June 18, 2020 00:29
@nturgut nturgut requested a review from yjbanov June 18, 2020 01:03
yjbanov
yjbanov reviewed Jun 18, 2020
View reviewed changes
@nturgut nturgut changed the title [web] Running integration tests on Safari on LUCI and Local [web] Running integration tests on Safari on Local Jun 18, 2020
@fluttergithubbot
Copy link
Contributor

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.

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

@nturgut
Copy link
Contributor Author

nturgut commented Jun 18, 2020

Merging the PR the error in the MaciOSEngine logs' step is not relevant to this PR's changes (This PR code path is Web Engine Test Harness)

@nturgut nturgut merged commit a358e99 into flutter:master Jun 18, 2020
@engine-flutter-autoroll engine-flutter-autoroll mentioned this pull request Jun 19, 2020
Closed
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Jun 19, 2020
@engine-flutter-autoroll
a358e99 [web] Running integration tests on Safari on Local (flutter/e…
e41d1ae 
…ngine#18488)
@engine-flutter-autoroll engine-flutter-autoroll mentioned this pull request Jun 19, 2020
Merged
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Jun 19, 2020
@engine-flutter-autoroll
a358e99 [web] Running integration tests on Safari on Local (flutter/e…
3c42a59 
…ngine#18488)
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers
1 more reviewer

@yjbanov yjbanov yjbanov approved these changes

Reviewers whose approvals may not affect merge requirements
Assignees
No one assigned
Labels
cla: yes
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Run integration tests on Safari-MacOS on Local
4 participants
@nturgut @fluttergithubbot @yjbanov @googlebot