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

Conversation

@nturgut
Copy link
Contributor

@nturgut nturgut commented Nov 21, 2020

Description

The tests on the semantics_test.dart was only running on chrome. This PR enables most of the unit test methods (excluding 2 of them) on safari, ios-safari and firefox

Related Issues

flutter/flutter#50590
flutter/flutter#46638

Tests

This PR is for enabling more unit tests.

Checklist

Before you create this PR confirm that it meets all requirements listed below by checking the relevant checkboxes ([x]). This will ensure a smooth and quick review process.

  • I read the contributor guide and followed the process outlined there for submitting PRs.
  • I signed the CLA.
  • I read and followed the C++, Objective-C, Java style guides for the engine.
  • I read the tree hygiene wiki page, which explains my responsibilities.
  • I updated/added relevant documentation.
  • All existing and new tests are passing.
  • I am willing to follow-up on review comments in a timely manner.

Reviewer Checklist

Breaking Change

Did any tests fail when you ran them? Please read handling breaking changes.

@nturgut nturgut requested a review from ferhatb November 21, 2020 00:20
@google-cla google-cla bot added the cla: yes label Nov 21, 2020
@nturgut nturgut requested a review from yjbanov November 21, 2020 00:23
// TODO(nurhan): https://github.com/flutter/flutter/issues/50754
skip: browserEngine == BrowserEngine.webkit ||
browserEngine == BrowserEngine.edge);
skip: browserEngine == BrowserEngine.edge);
Copy link
Contributor

Choose a reason for hiding this comment

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

This file isn't running on edge anyway.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That's true. I think I'll create another PR for removing all edge requirements though. Since our Edge testing code is for legacy edge. If we change it to new Edge all should pass (I hope :) )

@nturgut nturgut added the waiting for tree to go green This PR is approved and tested, but waiting for the tree to be green to land. label Nov 23, 2020
@nturgut nturgut merged commit fbf6818 into flutter:master Nov 23, 2020
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Nov 24, 2020
Copy link
Contributor

@ferhatb ferhatb left a comment

Choose a reason for hiding this comment

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

LGTM

chaselatta pushed a commit to chaselatta/engine that referenced this pull request Nov 30, 2020
…22662)

* enable safari tests for almost all the methods for semantics tests

* enable almost all semantics tests for firefox

* Safari desktop have different scroll max values for different versions

* fix assert
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

cla: yes waiting for tree to go green This PR is approved and tested, but waiting for the tree to be green to land.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants