Skip to content
Merged
Show file tree
Hide file tree
Changes from 2 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
74 changes: 63 additions & 11 deletions .github/workflows/dart.yml
Original file line number Diff line number Diff line change
Expand Up @@ -152,16 +152,16 @@ jobs:
if: "always() && steps.pkg_web_css_pub_get.conclusion == 'success'"
working-directory: pkg/web_css
job_003:
name: "smoke_test; PKG: pkg/indexed_blob; `dart format --output=none --set-exit-if-changed .`, `dart analyze --fatal-infos .`"
name: "smoke_test; PKGS: pkg/indexed_blob, pkg/screenshot_tools; `dart format --output=none --set-exit-if-changed .`, `dart analyze --fatal-infos .`"
runs-on: ubuntu-latest
steps:
- name: Cache Pub hosted dependencies
uses: actions/cache@d4323d4df104b026a6aa633fdb11d772146be0bf
with:
path: "~/.pub-cache/hosted"
key: "os:ubuntu-latest;pub-cache-hosted;sdk:3.9.0;packages:pkg/indexed_blob;commands:format-analyze_1"
key: "os:ubuntu-latest;pub-cache-hosted;sdk:3.9.0;packages:pkg/indexed_blob-pkg/screenshot_tools;commands:format-analyze_1"
restore-keys: |
os:ubuntu-latest;pub-cache-hosted;sdk:3.9.0;packages:pkg/indexed_blob
os:ubuntu-latest;pub-cache-hosted;sdk:3.9.0;packages:pkg/indexed_blob-pkg/screenshot_tools
os:ubuntu-latest;pub-cache-hosted;sdk:3.9.0
os:ubuntu-latest;pub-cache-hosted
os:ubuntu-latest
Expand All @@ -185,6 +185,19 @@ jobs:
run: dart analyze --fatal-infos .
if: "always() && steps.pkg_indexed_blob_pub_get.conclusion == 'success'"
working-directory: pkg/indexed_blob
- id: pkg_screenshot_tools_pub_get
name: pkg/screenshot_tools; dart pub get
run: dart pub get
if: "always() && steps.checkout.conclusion == 'success'"
working-directory: pkg/screenshot_tools
- name: "pkg/screenshot_tools; dart format --output=none --set-exit-if-changed ."
run: "dart format --output=none --set-exit-if-changed ."
if: "always() && steps.pkg_screenshot_tools_pub_get.conclusion == 'success'"
working-directory: pkg/screenshot_tools
- name: "pkg/screenshot_tools; dart analyze --fatal-infos ."
run: dart analyze --fatal-infos .
if: "always() && steps.pkg_screenshot_tools_pub_get.conclusion == 'success'"
working-directory: pkg/screenshot_tools
job_004:
name: "smoke_test; PKG: pkg/pub_integration; `dart format --output=none --set-exit-if-changed .`, `dart analyze --fatal-infos lib/`, `dart analyze --fatal-infos test/`"
runs-on: ubuntu-latest
Expand Down Expand Up @@ -872,6 +885,45 @@ jobs:
- job_007
- job_008
job_021:
name: "unit_test; PKG: pkg/screenshot_tools; `dart test --run-skipped`"
runs-on: ubuntu-latest
steps:
- name: Cache Pub hosted dependencies
uses: actions/cache@d4323d4df104b026a6aa633fdb11d772146be0bf
with:
path: "~/.pub-cache/hosted"
key: "os:ubuntu-latest;pub-cache-hosted;sdk:3.9.0;packages:pkg/screenshot_tools;commands:test_09"
restore-keys: |
os:ubuntu-latest;pub-cache-hosted;sdk:3.9.0;packages:pkg/screenshot_tools
os:ubuntu-latest;pub-cache-hosted;sdk:3.9.0
os:ubuntu-latest;pub-cache-hosted
os:ubuntu-latest
- name: Setup Dart SDK
uses: dart-lang/setup-dart@e51d8e571e22473a2ddebf0ef8a2123f0ab2c02c
with:
sdk: "3.9.0"
- id: checkout
name: Checkout repository
uses: actions/checkout@ac593985615ec2ede58e132d2e21d2b1cbd6127c
- id: pkg_screenshot_tools_pub_get
name: pkg/screenshot_tools; dart pub get
run: dart pub get
if: "always() && steps.checkout.conclusion == 'success'"
working-directory: pkg/screenshot_tools
- name: "pkg/screenshot_tools; dart test --run-skipped"
run: dart test --run-skipped
if: "always() && steps.pkg_screenshot_tools_pub_get.conclusion == 'success'"
working-directory: pkg/screenshot_tools
needs:
- job_001
- job_002
- job_003
- job_004
- job_005
- job_006
- job_007
- job_008
job_022:
name: "unit_test; PKG: pkg/web_app; `dart test --run-skipped`"
runs-on: ubuntu-latest
steps:
Expand Down Expand Up @@ -910,7 +962,7 @@ jobs:
- job_006
- job_007
- job_008
job_022:
job_023:
name: "unit_test; PKG: pkg/web_css; `dart test --run-skipped`"
runs-on: ubuntu-latest
steps:
Expand Down Expand Up @@ -949,7 +1001,7 @@ jobs:
- job_006
- job_007
- job_008
job_023:
job_024:
name: "unit_test; PKG: pkg/pub_integration; `dart test -j1 --run-skipped `find test -name \"*_test\\\\.dart\" | sort | sed -n '0~4p'``"
runs-on: ubuntu-latest
steps:
Expand Down Expand Up @@ -988,7 +1040,7 @@ jobs:
- job_006
- job_007
- job_008
job_024:
job_025:
name: "unit_test; PKG: pkg/pub_integration; `dart test -j1 --run-skipped `find test -name \"*_test\\\\.dart\" | sort | sed -n '1~4p'``"
runs-on: ubuntu-latest
steps:
Expand Down Expand Up @@ -1027,7 +1079,7 @@ jobs:
- job_006
- job_007
- job_008
job_025:
job_026:
name: "unit_test; PKG: pkg/pub_integration; `dart test -j1 --run-skipped `find test -name \"*_test\\\\.dart\" | sort | sed -n '2~4p'``"
runs-on: ubuntu-latest
steps:
Expand Down Expand Up @@ -1066,7 +1118,7 @@ jobs:
- job_006
- job_007
- job_008
job_026:
job_027:
name: "unit_test; PKG: pkg/pub_integration; `dart test -j1 --run-skipped `find test -name \"*_test\\\\.dart\" | sort | sed -n '3~4p'``"
runs-on: ubuntu-latest
steps:
Expand Down Expand Up @@ -1105,7 +1157,7 @@ jobs:
- job_006
- job_007
- job_008
job_027:
job_028:
name: "unit_test; PKG: pkg/pub_worker; `dart test --run-skipped --concurrency=1 `find test -name \"*_test\\\\.dart\" | sort | sed -n '0~3p'``"
runs-on: ubuntu-latest
steps:
Expand Down Expand Up @@ -1144,7 +1196,7 @@ jobs:
- job_006
- job_007
- job_008
job_028:
job_029:
name: "unit_test; PKG: pkg/pub_worker; `dart test --run-skipped --concurrency=1 `find test -name \"*_test\\\\.dart\" | sort | sed -n '1~3p'``"
runs-on: ubuntu-latest
steps:
Expand Down Expand Up @@ -1183,7 +1235,7 @@ jobs:
- job_006
- job_007
- job_008
job_029:
job_030:
name: "unit_test; PKG: pkg/pub_worker; `dart test --run-skipped --concurrency=1 `find test -name \"*_test\\\\.dart\" | sort | sed -n '2~3p'``"
runs-on: ubuntu-latest
steps:
Expand Down
30 changes: 0 additions & 30 deletions pkg/pub_integration/README.md

This file was deleted.

2 changes: 1 addition & 1 deletion pkg/pub_integration/lib/src/pub_puppeteer_helpers.dart
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,8 @@
import 'dart:async';

import 'package:puppeteer/puppeteer.dart';
import 'package:screenshot_tools/screenshot_utils.dart';
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: Should we change the name of the main library to align with the package name?

Suggested change
import 'package:screenshot_tools/screenshot_utils.dart';
import 'package:screenshot_tools/screenshot_tools.dart';

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I'll do the library name changes in a follow-up, after more changes has been done, e.g. the dartdoc-related tooling is also implemented.

Copy link
Contributor

Choose a reason for hiding this comment

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

And should we have the name of the new package indicate somehow what it is screenshotting?

Something like:

Suggested change
import 'package:screenshot_tools/screenshot_utils.dart';
import 'package:browser_screenshots/browser_screenshots.dart';

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

How about puppeteer_screenshots?


import 'screenshot_utils.dart';
import 'test_browser.dart';

const webmastersReadonlyScope =
Expand Down
3 changes: 1 addition & 2 deletions pkg/pub_integration/lib/src/test_browser.dart
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,7 @@ import 'package:_pub_shared/validation/html/html_validation.dart';
import 'package:path/path.dart' as p;
import 'package:puppeteer/puppeteer.dart';
import 'package:retry/retry.dart';

import 'screenshot_utils.dart';
import 'package:screenshot_tools/screenshot_utils.dart';

/// Creates and tracks the headless Chrome environment, its temp directories and
/// and uncaught exceptions.
Expand Down
2 changes: 1 addition & 1 deletion pkg/pub_integration/pubspec.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -14,11 +14,11 @@ dependencies:
pub_semver: ^2.0.0
puppeteer: 3.16.0
retry: ^3.1.2
screenshot_tools:
oauth2: ^2.0.0

dev_dependencies:
coverage: any # test already depends on it
markdown: ^7.3.0
pubspec_parse: ^1.5.0
shelf: ^1.4.0
test: ^1.16.5
2 changes: 1 addition & 1 deletion pkg/pub_integration/test/browser_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -7,9 +7,9 @@ import 'dart:convert';
import 'package:http/http.dart' as http;
import 'package:pub_integration/src/fake_test_context_provider.dart';
import 'package:pub_integration/src/pub_puppeteer_helpers.dart';
import 'package:pub_integration/src/screenshot_utils.dart';
import 'package:pub_integration/src/test_browser.dart';
import 'package:puppeteer/puppeteer.dart';
import 'package:screenshot_tools/screenshot_utils.dart';
import 'package:test/test.dart';

void main() {
Expand Down
2 changes: 1 addition & 1 deletion pkg/pub_integration/test/dartdoc_search_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,9 @@ import 'dart:convert';

import 'package:http/http.dart' as http;
import 'package:pub_integration/src/fake_test_context_provider.dart';
import 'package:pub_integration/src/screenshot_utils.dart';
import 'package:pub_integration/src/test_browser.dart';
import 'package:puppeteer/puppeteer.dart';
import 'package:screenshot_tools/screenshot_utils.dart';
import 'package:test/test.dart';

void main() {
Expand Down
2 changes: 1 addition & 1 deletion pkg/pub_integration/test/fake_sign_in_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,8 @@ import 'dart:convert';

import 'package:http/http.dart' as http;
import 'package:pub_integration/src/fake_test_context_provider.dart';
import 'package:pub_integration/src/screenshot_utils.dart';
import 'package:pub_integration/src/test_browser.dart';
import 'package:screenshot_tools/screenshot_utils.dart';
import 'package:test/test.dart';

void main() {
Expand Down
2 changes: 1 addition & 1 deletion pkg/pub_integration/test/pkg_admin_page_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,8 @@ import 'dart:io' show Platform;
import 'package:http/http.dart' as http;
import 'package:pub_integration/src/fake_test_context_provider.dart';
import 'package:pub_integration/src/pub_puppeteer_helpers.dart';
import 'package:pub_integration/src/screenshot_utils.dart';
import 'package:pub_integration/src/test_browser.dart';
import 'package:screenshot_tools/screenshot_utils.dart';
import 'package:test/test.dart';

void main() {
Expand Down
2 changes: 1 addition & 1 deletion pkg/pub_integration/test/report_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,8 @@ import 'package:_pub_shared/data/admin_api.dart';
import 'package:http/http.dart' as http;
import 'package:pub_integration/src/fake_test_context_provider.dart';
import 'package:pub_integration/src/pub_puppeteer_helpers.dart';
import 'package:pub_integration/src/screenshot_utils.dart';
import 'package:pub_integration/src/test_browser.dart';
import 'package:screenshot_tools/screenshot_utils.dart';
import 'package:test/test.dart';

final _caseIdExpr = RegExp(r'[0-9]{8}I[0-9a-f]{10}');
Expand Down
2 changes: 1 addition & 1 deletion pkg/pub_integration/test/search_completition_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,9 @@ import 'dart:convert';

import 'package:http/http.dart' as http;
import 'package:pub_integration/src/fake_test_context_provider.dart';
import 'package:pub_integration/src/screenshot_utils.dart';
import 'package:pub_integration/src/test_browser.dart';
import 'package:puppeteer/puppeteer.dart';
import 'package:screenshot_tools/screenshot_utils.dart';
import 'package:test/test.dart';

void main() {
Expand Down
2 changes: 1 addition & 1 deletion pkg/pub_integration/test/search_update_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -7,9 +7,9 @@ import 'dart:convert';
import 'package:http/http.dart' as http;
import 'package:pub_integration/src/fake_test_context_provider.dart';
import 'package:pub_integration/src/pub_puppeteer_helpers.dart';
import 'package:pub_integration/src/screenshot_utils.dart';
import 'package:pub_integration/src/test_browser.dart';
import 'package:puppeteer/puppeteer.dart';
import 'package:screenshot_tools/screenshot_utils.dart';
import 'package:test/test.dart';

void main() {
Expand Down
38 changes: 38 additions & 0 deletions pkg/screenshot_tools/README.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
# Screenshots

This tool helps create and compare screenshots before/after changes that affect visual styles.

## Creating screenshots

The screenshots are created during the `puppeteer` tests, if the `SCREENSHOT_DIR`
environment variable is set.

The tests are instrumented with calls to take screenshots on selected parts of
the screens. During the method call, it will change the viewport and the main
theme of the page, taking 6 screenshots for each call.

## Before/after screenshots

To create before/after screenshot comparisons, use the following workflow:

1. Generate screenshots by running tests with `SCREENSHOT_DIR=/path/to/before/dir`.
1. Make the changes.
1. Generate screenshots by running tests with `SCREENSHOT_DIR=/path/to/after/dir`.
1. Run `dart bin/compare_screenshots.dart <before-dir> <after-dir> <diff-dir>`

The comparison will put the files into the `<diff-dir>` with the before/after/diff
versions, alongside a summary `index.html` file to have a quick overview of them.

### Required tool for comparisons

The comparison is created by the `odiff` tool running in a container using `docker`.
To build the `odiff` container, user the following command:

```
cd tool
docker build . -f Dockerfile.odiff -t odiff
```

## Missing features

- Create a better side-by-side comparison for images, as right now everything is only a list of images.
Loading