Enable web engine unit tests on Firefox (flutter#14267)

* Enable web engine unit tests on Firefox * addressing PR comments * addressing PR comments * fix the version name on the lock file
elancom · Dec 10, 2019 · 2805da9 · 2805da9
1 parent b2ab78f
commit 2805da9
Show file tree

Hide file tree

Showing 12 changed files with 105 additions and 69 deletions.
diff --git a/.cirrus.yml b/.cirrus.yml
@@ -75,8 +75,8 @@ task:
         $ENGINE_PATH/src/out/host_debug_unopt/dart-sdk/bin/pub get
         cd $ENGINE_PATH/src/flutter/lib/web_ui
         $ENGINE_PATH/src/out/host_debug_unopt/dart-sdk/bin/pub get
-        export DART="$ENGINE_PATH/src/out/host_debug_unopt/dart-sdk/bin/dart"
-        $DART dev/firefox_installer_test.dart
+        export FELT="$ENGINE_PATH/src/out/host_debug_unopt/dart-sdk/bin/dart dev/felt.dart"
+        $FELT test --browser=firefox
     - name: build_and_test_android_unopt_debug
       env:
         USE_ANDROID: "True"

diff --git a/lib/web_ui/dev/browser_lock.yaml b/lib/web_ui/dev/browser_lock.yaml
@@ -4,4 +4,4 @@ chrome:
   Linux: 695653
   Mac: 695656
 firefox:
-  version: 69.0.3
+  version: '71.0'
diff --git a/lib/web_ui/dev/firefox.dart b/lib/web_ui/dev/firefox.dart
@@ -48,12 +48,11 @@ class Firefox extends Browser {
       var dir = createTempDir();
       var args = [
         url.toString(),
-        if (!debug) '--headless',
+        '--headless',
         '-width $kMaxScreenshotWidth',
         '-height $kMaxScreenshotHeight',
         '-new-window',
         '-new-instance',
-        '-prefs { "dom.disable_beforeunload" = true, "toolkit.startup.max_resumed_crashes"=999999 } ',
         '--start-debugger-server $kDevtoolsPort',
       ];
 

diff --git a/lib/web_ui/dev/test_runner.dart b/lib/web_ui/dev/test_runner.dart
@@ -83,6 +83,8 @@ class TestCommand extends Command<bool> {
 
   String get browser => argResults['browser'];
 
+  bool get isChrome => argResults['browser'] == 'chrome';
+
   /// When running screenshot tests writes them to the file system into
   /// ".dart_tool/goldens".
   bool get doUpdateScreenshotGoldens => argResults['update-screenshot-goldens'];
@@ -98,54 +100,74 @@ class TestCommand extends Command<bool> {
       'test',
     ));
 
-    // Separate screenshot tests from unit-tests. Screenshot tests must run
-    // one at a time. Otherwise, they will end up screenshotting each other.
-    // This is not an issue for unit-tests.
-    final FilePath failureSmokeTestPath = FilePath.fromWebUi(
-      'test/golden_tests/golden_failure_smoke_test.dart',
-    );
-    final List<FilePath> screenshotTestFiles = <FilePath>[];
-    final List<FilePath> unitTestFiles = <FilePath>[];
-
-    for (io.File testFile
-        in testDir.listSync(recursive: true).whereType<io.File>()) {
-      final FilePath testFilePath = FilePath.fromCwd(testFile.path);
-      if (!testFilePath.absolute.endsWith('_test.dart')) {
-        // Not a test file at all. Skip.
-        continue;
-      }
-      if (testFilePath == failureSmokeTestPath) {
-        // A smoke test that fails on purpose. Skip.
-        continue;
+    // Screenshot tests and smoke tests only run in Chrome.
+    if (isChrome) {
+      // Separate screenshot tests from unit-tests. Screenshot tests must run
+      // one at a time. Otherwise, they will end up screenshotting each other.
+      // This is not an issue for unit-tests.
+      final FilePath failureSmokeTestPath = FilePath.fromWebUi(
+        'test/golden_tests/golden_failure_smoke_test.dart',
+      );
+      final List<FilePath> screenshotTestFiles = <FilePath>[];
+      final List<FilePath> unitTestFiles = <FilePath>[];
+
+      for (io.File testFile
+          in testDir.listSync(recursive: true).whereType<io.File>()) {
+        final FilePath testFilePath = FilePath.fromCwd(testFile.path);
+        if (!testFilePath.absolute.endsWith('_test.dart')) {
+          // Not a test file at all. Skip.
+          continue;
+        }
+        if (testFilePath == failureSmokeTestPath) {
+          // A smoke test that fails on purpose. Skip.
+          continue;
+        }
+
+        if (path.split(testFilePath.relativeToWebUi).contains('golden_tests')) {
+          screenshotTestFiles.add(testFilePath);
+        } else {
+          unitTestFiles.add(testFilePath);
+        }
       }
-      if (path.split(testFilePath.relativeToWebUi).contains('golden_tests')) {
-        screenshotTestFiles.add(testFilePath);
-      } else {
-        unitTestFiles.add(testFilePath);
+
+      // This test returns a non-zero exit code on purpose. Run it separately.
+      if (io.Platform.environment['CIRRUS_CI'] != 'true') {
+        await _runTestBatch(
+          <FilePath>[failureSmokeTestPath],
+          concurrency: 1,
+          expectFailure: true,
+        );
+        _checkExitCode();
       }
-    }
 
-    // This test returns a non-zero exit code on purpose. Run it separately.
-    if (io.Platform.environment['CIRRUS_CI'] != 'true') {
-      await _runTestBatch(
-        <FilePath>[failureSmokeTestPath],
-        concurrency: 1,
-        expectFailure: true,
-      );
+      // Run all unit-tests as a single batch.
+      await _runTestBatch(unitTestFiles, concurrency: 10, expectFailure: false);
       _checkExitCode();
-    }
-
-    // Run all unit-tests as a single batch.
-    await _runTestBatch(unitTestFiles, concurrency: 10, expectFailure: false);
-    _checkExitCode();
 
-    // Run screenshot tests one at a time.
-    for (FilePath testFilePath in screenshotTestFiles) {
-      await _runTestBatch(
-        <FilePath>[testFilePath],
-        concurrency: 1,
-        expectFailure: false,
-      );
+      // Run screenshot tests one at a time.
+      for (FilePath testFilePath in screenshotTestFiles) {
+        await _runTestBatch(
+          <FilePath>[testFilePath],
+          concurrency: 1,
+          expectFailure: false,
+        );
+        _checkExitCode();
+      }
+    } else {
+      final List<FilePath> unitTestFiles = <FilePath>[];
+      for (io.File testFile
+          in testDir.listSync(recursive: true).whereType<io.File>()) {
+        final FilePath testFilePath = FilePath.fromCwd(testFile.path);
+        if (!testFilePath.absolute.endsWith('_test.dart')) {
+          // Not a test file at all. Skip.
+          continue;
+        }
+        if(!path.split(testFilePath.relativeToWebUi).contains('golden_tests')) {
+          unitTestFiles.add(testFilePath);
+        }
+      }
+      // Run all unit-tests as a single batch.
+      await _runTestBatch(unitTestFiles, concurrency: 10, expectFailure: false);
       _checkExitCode();
     }
   }

diff --git a/lib/web_ui/test/compositing_test.dart b/lib/web_ui/test/compositing_test.dart
@@ -173,7 +173,8 @@ void main() {
       expect(picture.buildCount, 1);
       expect(picture.updateCount, 0);
       expect(picture.applyPaintCount, 2);
-    });
+    }, // TODO(nurhan): https://github.com/flutter/flutter/issues/46638
+        skip: (browserEngine == BrowserEngine.firefox));
   });
 }
 

diff --git a/lib/web_ui/test/dom_renderer_test.dart b/lib/web_ui/test/dom_renderer_test.dart
@@ -115,7 +115,8 @@ void main() {
 
     final DomRenderer renderer = DomRenderer();
     renderer.reset();
-  });
+  }, // TODO(nurhan): https://github.com/flutter/flutter/issues/46638
+      skip: (browserEngine == BrowserEngine.firefox));
 
   test('accesibility placeholder is attached after creation', () {
     DomRenderer();

diff --git a/lib/web_ui/test/engine/semantics/semantics_helper_test.dart b/lib/web_ui/test/engine/semantics/semantics_helper_test.dart
@@ -21,7 +21,7 @@ void main() {
       if (_placeholder != null) {
         _placeholder.remove();
       }
-      if(desktopSemanticsEnabler?.semanticsActivationTimer != null) {
+      if (desktopSemanticsEnabler?.semanticsActivationTimer != null) {
         desktopSemanticsEnabler.semanticsActivationTimer.cancel();
         desktopSemanticsEnabler.semanticsActivationTimer = null;
       }
@@ -142,6 +142,7 @@ void main() {
           mobileSemanticsEnabler.tryEnableSemantics(event);
 
       expect(shouldForwardToFramework, true);
-    });
+    }, // TODO(nurhan): https://github.com/flutter/flutter/issues/46638
+        skip: (browserEngine == BrowserEngine.firefox));
   });
 }
diff --git a/lib/web_ui/test/engine/semantics/semantics_test.dart b/lib/web_ui/test/engine/semantics/semantics_test.dart
@@ -806,7 +806,8 @@ void _testTextField() {
     expect(await logger.actionLog.first, ui.SemanticsAction.tap);
 
     semantics().semanticsEnabled = false;
-  });
+  }, // TODO(nurhan): https://github.com/flutter/flutter/issues/46638
+      skip: (browserEngine == BrowserEngine.firefox));
 }
 
 void _testCheckables() {

diff --git a/lib/web_ui/test/paragraph_test.dart b/lib/web_ui/test/paragraph_test.dart
@@ -7,15 +7,15 @@ import 'package:ui/ui.dart';
 
 import 'package:test/test.dart';
 
-void testEachMeasurement(String description, VoidCallback body) {
+void testEachMeasurement(String description, VoidCallback body, {bool skip}) {
   test(description, () async {
     try {
       TextMeasurementService.initialize(rulerCacheCapacity: 2);
       return body();
     } finally {
       TextMeasurementService.clearCache();
     }
-  });
+  }, skip: skip);
   test('$description (canvas measurement)', () async {
     try {
       TextMeasurementService.initialize(rulerCacheCapacity: 2);
@@ -25,7 +25,7 @@ void testEachMeasurement(String description, VoidCallback body) {
       TextMeasurementService.enableExperimentalCanvasImplementation = false;
       TextMeasurementService.clearCache();
     }
-  });
+  }, skip: skip);
 }
 
 void main() async {
@@ -104,7 +104,8 @@ void main() async {
       expect(paragraph.minIntrinsicWidth, fontSize * 10.0);
       expect(paragraph.maxIntrinsicWidth, fontSize * 10.0);
     }
-  });
+  }, // TODO(nurhan): https://github.com/flutter/flutter/issues/46638
+      skip: (browserEngine == BrowserEngine.firefox));
 
   testEachMeasurement('predictably lays out a multi-line rich paragraph', () {
     for (double fontSize in <double>[10.0, 20.0, 30.0, 40.0]) {
@@ -126,7 +127,8 @@ void main() async {
       expect(paragraph.minIntrinsicWidth, fontSize * 5.0);
       expect(paragraph.maxIntrinsicWidth, fontSize * 16.0);
     }
-  });
+  }, // TODO(nurhan): https://github.com/flutter/flutter/issues/46638
+      skip: (browserEngine == BrowserEngine.firefox));
 
   testEachMeasurement('getBoxesForRange returns a box', () {
     final ParagraphBuilder builder = ParagraphBuilder(ParagraphStyle(

diff --git a/lib/web_ui/test/text/font_collection_test.dart b/lib/web_ui/test/text/font_collection_test.dart
@@ -84,7 +84,7 @@ void main() {
       expect(fontFamilyList.length, equals(2));
       expect(fontFamilyList, contains('\'/Ahem\''));
       expect(fontFamilyList, contains('/Ahem'));
-    });
+    }, skip: (browserEngine == BrowserEngine.firefox));
 
     test('Register Asset twice with exclamation mark', () async {
       final String _testFontFamily = 'Ahem!!ahem';
@@ -101,7 +101,7 @@ void main() {
       expect(fontFamilyList.length, equals(2));
       expect(fontFamilyList, contains('\'Ahem!!ahem\''));
       expect(fontFamilyList, contains('Ahem!!ahem'));
-    });
+    }, skip: (browserEngine == BrowserEngine.firefox));
 
     test('Register Asset twice with coma', () async {
       final String _testFontFamily = 'Ahem ,ahem';
@@ -118,7 +118,8 @@ void main() {
       expect(fontFamilyList.length, equals(2));
       expect(fontFamilyList, contains('\'Ahem ,ahem\''));
       expect(fontFamilyList, contains('Ahem ,ahem'));
-    });
+    }, // TODO(nurhan): https://github.com/flutter/flutter/issues/46638
+        skip: (browserEngine == BrowserEngine.firefox));
 
     test('Register Asset twice with a digit at the start of a token', () async {
       final String testFontFamily = 'Ahem 1998';
@@ -136,5 +137,6 @@ void main() {
       expect(fontFamilyList, contains('Ahem 1998'));
       expect(fontFamilyList, contains('\'Ahem 1998\''));
     });
-  });
+  }, // TODO(nurhan): https://github.com/flutter/flutter/issues/46638
+      skip: (browserEngine == BrowserEngine.firefox));
 }
diff --git a/lib/web_ui/test/text_editing_test.dart b/lib/web_ui/test/text_editing_test.dart
@@ -186,7 +186,8 @@ void main() {
 
       // There should be no input action.
       expect(lastInputAction, isNull);
-    });
+    }, // TODO(nurhan): https://github.com/flutter/flutter/issues/46638
+        skip: (browserEngine == BrowserEngine.firefox));
 
     test('Can set editing state correctly', () {
       editingElement.enable(
@@ -217,7 +218,8 @@ void main() {
 
       // There should be no input action.
       expect(lastInputAction, isNull);
-    });
+    }, // TODO(nurhan): https://github.com/flutter/flutter/issues/46638
+        skip: (browserEngine == BrowserEngine.firefox));
 
     test('Multi-line mode also works', () {
       // The textarea element is created lazily.
@@ -262,7 +264,8 @@ void main() {
 
       // There should be no input action.
       expect(lastInputAction, isNull);
-    });
+    }, // TODO(nurhan): https://github.com/flutter/flutter/issues/46638
+        skip: (browserEngine == BrowserEngine.firefox));
 
     test('Same instance can be re-enabled with different config', () {
       // Make sure there's nothing in the DOM yet.
@@ -922,7 +925,8 @@ void main() {
       );
 
       hideKeyboard();
-    });
+    }, // TODO(nurhan): https://github.com/flutter/flutter/issues/46638
+        skip: (browserEngine == BrowserEngine.firefox));
 
     test('Multi-line mode also works', () {
       final MethodCall setClient = MethodCall(
@@ -980,7 +984,8 @@ void main() {
 
       // Confirm that [HybridTextEditing] didn't send any more messages.
       expect(spy.messages, isEmpty);
-    });
+    }, // TODO(nurhan): https://github.com/flutter/flutter/issues/46638
+        skip: (browserEngine == BrowserEngine.firefox));
 
     test('sets correct input type in Android', () {
       debugOperatingSystemOverride = OperatingSystem.android;

diff --git a/lib/web_ui/test/text_test.dart b/lib/web_ui/test/text_test.dart
@@ -244,7 +244,8 @@ void main() async {
     expect(spans[0].style.fontFamily, 'Ahem, Arial, sans-serif');
     // The nested span here should not set it's family to default sans-serif.
     expect(spans[1].style.fontFamily, 'Ahem, Arial, sans-serif');
-  });
+  }, // TODO(nurhan): https://github.com/flutter/flutter/issues/46638
+      skip: (browserEngine == BrowserEngine.firefox));
 
   test('adds Arial and sans-serif as fallback fonts', () {
     // Set this to false so it doesn't default to 'Ahem' font.
@@ -261,7 +262,8 @@ void main() async {
     expect(paragraph.paragraphElement.style.fontFamily, 'SomeFont, Arial, sans-serif');
 
     debugEmulateFlutterTesterEnvironment = true;
-  });
+  }, // TODO(nurhan): https://github.com/flutter/flutter/issues/46638
+      skip: (browserEngine == BrowserEngine.firefox));
 
   test('does not add fallback fonts to generic families', () {
     // Set this to false so it doesn't default to 'Ahem' font.