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

Enable web engine unit tests on Firefox #14267

Merged
merged 4 commits into from
Dec 10, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions .cirrus.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down
2 changes: 1 addition & 1 deletion lib/web_ui/dev/browser_lock.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -4,4 +4,4 @@ chrome:
Linux: 695653
Mac: 695656
firefox:
version: 69.0.3
version: '71.0'
3 changes: 1 addition & 2 deletions lib/web_ui/dev/firefox.dart
Original file line number Diff line number Diff line change
Expand Up @@ -48,12 +48,11 @@ class Firefox extends Browser {
var dir = createTempDir();
var args = [
url.toString(),
if (!debug) '--headless',
'--headless',
nturgut marked this conversation as resolved.
Show resolved Hide resolved
'-width $kMaxScreenshotWidth',
'-height $kMaxScreenshotHeight',
'-new-window',
'-new-instance',
'-prefs { "dom.disable_beforeunload" = true, "toolkit.startup.max_resumed_crashes"=999999 } ',
'--start-debugger-server $kDevtoolsPort',
];

Expand Down
108 changes: 65 additions & 43 deletions lib/web_ui/dev/test_runner.dart
Original file line number Diff line number Diff line change
Expand Up @@ -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'];
Expand All @@ -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();
}
}
Expand Down
3 changes: 2 additions & 1 deletion lib/web_ui/test/compositing_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -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));
});
}

Expand Down
3 changes: 2 additions & 1 deletion lib/web_ui/test/dom_renderer_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down
5 changes: 3 additions & 2 deletions lib/web_ui/test/engine/semantics/semantics_helper_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand Down Expand Up @@ -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));
});
}
3 changes: 2 additions & 1 deletion lib/web_ui/test/engine/semantics/semantics_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -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() {
Expand Down
12 changes: 7 additions & 5 deletions lib/web_ui/test/paragraph_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand All @@ -25,7 +25,7 @@ void testEachMeasurement(String description, VoidCallback body) {
TextMeasurementService.enableExperimentalCanvasImplementation = false;
TextMeasurementService.clearCache();
}
});
}, skip: skip);
}

void main() async {
Expand Down Expand Up @@ -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]) {
Expand All @@ -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(
Expand Down
10 changes: 6 additions & 4 deletions lib/web_ui/test/text/font_collection_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand All @@ -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';
Expand All @@ -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';
Expand All @@ -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));
}
15 changes: 10 additions & 5 deletions lib/web_ui/test/text_editing_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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(
Expand Down Expand Up @@ -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;
Expand Down
6 changes: 4 additions & 2 deletions lib/web_ui/test/text_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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.
Expand Down