Skip to content

Take screenshots from pub_integration tests (store locally). #8575

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.

Sign up for GitHub

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

Merged
merged 1 commit into from
Feb 20, 2025
Merged

Take screenshots from pub_integration tests (store locally). #8575

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
8 changes: 2 additions & 6 deletions pkg/pub_integration/lib/src/pub_puppeteer_helpers.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,10 @@
// for details. All rights reserved. Use of this source code is governed by a
// BSD-style license that can be found in the LICENSE file.
import 'dart:async';
import 'dart:io';

import 'package:puppeteer/puppeteer.dart';

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

const webmastersReadonlyScope =
Expand Down Expand Up @@ -46,10 +46,6 @@ Future<ListingPageInfo> listingPageInfo(Page page) async {
}

extension PubPageExt on Page {
Future<void> saveScreenshot(String path) async {
await File(path).writeAsBytes(await screenshot());
}

Future<void> waitFocusAndType(String selector, String text) async {
await waitForSelector(selector, timeout: Duration(seconds: 5));
await focus(selector);
Expand Down Expand Up @@ -221,7 +217,7 @@ extension PubPageExt on Page {
return;
}
}
await saveScreenshot('layout-timeout.png');
await writeScreenshotToFile('layout-timeout.png');
throw TimeoutException('Did not have a stable layout in $timeout.');
}

Expand Down
102 changes: 102 additions & 0 deletions pkg/pub_integration/lib/src/screenshot_utils.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,102 @@
// Copyright (c) 2024, the Dart project authors. Please see the AUTHORS file
// for details. All rights reserved. Use of this source code is governed by a
// BSD-style license that can be found in the LICENSE file.

import 'dart:io';

import 'package:path/path.dart' as p;
import 'package:puppeteer/puppeteer.dart';

// Default screen with 16:10 ratio.
final desktopDeviceViewport = DeviceViewport(width: 1280, height: 800);

final _screenshotDir = Platform.environment['PUB_SCREENSHOT_DIR'];
Copy link
Member

@jonasfj jonasfj Feb 19, 2025

Choose a reason for hiding this comment

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

How about we just always store the screenshots in .dart_tool/screenshots/.

  • The .dart_tool/ folder is already there.
  • The .dart_tool/ folder is always in .gitignore.

Copy link
Member

@jonasfj jonasfj Feb 19, 2025

Choose a reason for hiding this comment

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

Borderline crazy idea:

.dart_tool/
  screenshots/
    <revision>/
      image1.png
    current/
      image1.png
    comparison.html

Whenever, we create screenshots we:

  • Always write to .dart_tool/screenshots/current/
  • If git status --porcelain (meaning we have no changes), then:
    • We also write to .dart_tool/screenshots/<revision>/
  • We always create .dart_tool/screenshots/comparison.html
    • A simple dump HTML file showing screenshots side by side
    • Contents of .dart_tool/screenshots/current/ on the right side
    • Contents of <revision> on the left side, where you pick <revision> from a dropdown
      • We create a dropdown at the top of the HTML file.
      • We run git log to find relevant revisions
        • We put relevant revisions in dropdown, if .dart_tool/screenshots/<revision>/ exists locally.
        • We order the dropdown by git log, and default to the most recent revision we have.

We could also use gh api to download artifacts from github:
https://docs.github.com/en/rest/actions/artifacts?apiVersion=2022-11-28#get-an-artifact

Note

  • An artifact on github is a zip folder.
  • It's not possible to download an artifact without being authenticated!
  • Artifacts can't be embedded in comments as images :(

If wanted to have before/after images on github, we'll probably have to do something borderline crazy like uploading the images to a separate branch or a GCS bucket.
Which probably involve more infrastructure than we care to stand up and maintain.

Copy link
Collaborator Author

@isoos isoos Feb 19, 2025

Choose a reason for hiding this comment

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

Not a crazy idea, but let's not rush to it yet, and reach it maybe step-by-step. There is an immediate benefit with this + a comparison tool, and then we can add further automatization with github artifacts and/or per-revision storage...

final _isScreenshotDirSet =
_screenshotDir != null && _screenshotDir!.isNotEmpty;

// Set this variable to enable screenshot files to be updated with new takes.
// The default is to throw an exception to prevent accidental overrides from
// separate tests.
final _allowScreeshotUpdates =
Platform.environment['PUB_SCREENSHOT_UPDATE'] == '1';

// Note: The default values are the last, so we don't need reset
// the original values after taking the screenshots.
final _themes = ['dark', 'light'];
final _viewports = {
'mobile': DeviceViewport(width: 400, height: 800),
'tablet': DeviceViewport(width: 768, height: 1024),
Copy link
Member

@jonasfj jonasfj Feb 19, 2025

Choose a reason for hiding this comment

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

Might I propose we don't do tablet (just because of the number of screenshots)? Also don't those normally have much higher resolution (same with phones).

I'm assuming this is more about aspect ratio?

Copy link
Collaborator Author

@isoos isoos Feb 19, 2025

Choose a reason for hiding this comment

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

Why don't do tablet? I think it is a good way to check how the site behaves in narrower screen, and tbh. I don't think we will manually check tablet screens, but may catch if something degrades.

Copy link
Collaborator Author

@isoos isoos Feb 19, 2025

Choose a reason for hiding this comment

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

I'm assuming this is more about aspect ratio?

Mostly the width.

Copy link
Member

@jonasfj jonasfj Feb 19, 2025

Choose a reason for hiding this comment

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

I'm mostly thinking that we already have a LOT of screenshots, so I don't mind it, I'm only concerned that it's hard to get an overview.

Copy link
Collaborator Author

@isoos isoos Feb 19, 2025

Choose a reason for hiding this comment

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

Let's keep them for now, in the hope they are useful. If they are not, create too much noise, we may remove them or ignore them in most comparison reports.

'desktop': desktopDeviceViewport,
};

extension ScreenshotPageExt on Page {
Future<void> writeScreenshotToFile(String path) async {
await File(path).writeAsBytes(await screenshot());
}

/// Takes screenshots **if** `PUB_SCREENSHOT_DIR` environment variable is set.
///
/// Iterates over viewports and themes, and generates screenshot files with the
/// following pattern:
/// - `PUB_SCREENSHOT_DIR/$prefix-desktop-dark.png`
/// - `PUB_SCREENSHOT_DIR/$prefix-desktop-light.png`
/// - `PUB_SCREENSHOT_DIR/$prefix-mobile-dark.png`
/// - `PUB_SCREENSHOT_DIR/$prefix-mobile-light.png`
/// - `PUB_SCREENSHOT_DIR/$prefix-tablet-dark.png`
/// - `PUB_SCREENSHOT_DIR/$prefix-tablet-light.png`
Future<void> takeScreenshots({
required String selector,
required String prefix,
}) async {
final handle = await $(selector);
await handle.takeScreenshots(prefix);
}
}

extension ScreenshotElementHandleExt on ElementHandle {
/// Takes screenshots **if** `PUB_SCREENSHOT_DIR` environment variable is set.
///
/// Iterates over viewports and themes, and generates screenshot files with the
/// following pattern:
/// - `PUB_SCREENSHOT_DIR/$prefix-desktop-dark.png`
/// - `PUB_SCREENSHOT_DIR/$prefix-desktop-light.png`
/// - `PUB_SCREENSHOT_DIR/$prefix-mobile-dark.png`
/// - `PUB_SCREENSHOT_DIR/$prefix-mobile-light.png`
/// - `PUB_SCREENSHOT_DIR/$prefix-tablet-dark.png`
/// - `PUB_SCREENSHOT_DIR/$prefix-tablet-light.png`
Future<void> takeScreenshots(String prefix) async {
final body = await page.$('body');
final bodyClassAttr =
(await body.evaluate('el => el.getAttribute("class")')) as String;
final bodyClasses = bodyClassAttr.split(' ');

for (final vp in _viewports.entries) {
await page.setViewport(vp.value);

for (final theme in _themes) {
final newClasses = [
...bodyClasses.where((c) => !c.endsWith('-theme')),
'$theme-theme',
];
await body.evaluate('(el, v) => el.setAttribute("class", v)',
args: [newClasses.join(' ')]);

// The presence of the element is verified, continue only if screenshots are enabled.
if (!_isScreenshotDirSet) continue;

final path = p.join(_screenshotDir!, '$prefix-${vp.key}-$theme.png');
await _writeScreenshotToFile(path);
}
}
}

Future<void> _writeScreenshotToFile(String path) async {
final file = File(path);
final exists = await file.exists();
if (exists && !_allowScreeshotUpdates) {
throw Exception('Screenshot update is detected in: $path');
}
await file.parent.create(recursive: true);
await File(path).writeAsBytes(await screenshot());
}
}
3 changes: 3 additions & 0 deletions pkg/pub_integration/lib/src/test_browser.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,8 @@ import 'package:_pub_shared/validation/html/html_validation.dart';
import 'package:path/path.dart' as p;
import 'package:puppeteer/puppeteer.dart';

import 'screenshot_utils.dart';

/// Creates and tracks the headless Chrome environment, its temp directories and
/// and uncaught exceptions.
class TestBrowser {
Expand Down Expand Up @@ -90,6 +92,7 @@ class TestBrowser {
userDataDir: userDataDir.path,
headless: !_displayBrowser,
devTools: false,
defaultViewport: desktopDeviceViewport,
);

// Update the default permissions like clipboard access.
Expand Down
11 changes: 11 additions & 0 deletions pkg/pub_integration/test/browser_test.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ 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:test/test.dart';
Expand Down Expand Up @@ -69,6 +70,8 @@ void main() {
await user.withBrowserPage(
(page) async {
await page.gotoOrigin('/packages/retry');
await page.takeScreenshots(
prefix: 'package-page/readme-page', selector: 'body');

// check pub score
final pubScoreElem = await page
Expand All @@ -94,6 +97,14 @@ void main() {

await page.gotoOrigin('/packages/retry/license');
await checkHeaderTitle();

await page.gotoOrigin('/packages/retry/versions');
await page.takeScreenshots(
prefix: 'package-page/versions-page', selector: 'body');

await page.gotoOrigin('/packages/retry/score');
await page.takeScreenshots(
prefix: 'package-page/score-page', selector: 'body');
},
);
});
Expand Down
11 changes: 11 additions & 0 deletions pkg/pub_integration/test/fake_sign_in_test.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ 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:test/test.dart';

Expand Down Expand Up @@ -50,6 +51,13 @@ void main() {
String? firstSessionId;
// sign-in page
await browserSession.withBrowserPage((page) async {
await page.gotoOrigin('/');
await page.takeScreenshots(
selector: '.site-header',
prefix: 'landing-page/site-header-public');
await page.takeScreenshots(
selector: '.site-footer', prefix: 'landing-page/site-footer');

{
final rs = await page.gotoOrigin('/sign-in?fake-email=user@pub.dev');
final cookies = await page.cookies();
Expand All @@ -63,6 +71,9 @@ void main() {
firstSessionId =
cookies.firstWhere((c) => c.name == 'PUB_SID_INSECURE').value;
}
await page.takeScreenshots(
selector: '.site-header',
prefix: 'landing-page/site-header-authenticated');

// same user sign-in with redirect
{
Expand Down
3 changes: 3 additions & 0 deletions pkg/pub_integration/test/pkg_admin_page_test.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ 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:test/test.dart';

Expand Down Expand Up @@ -52,6 +53,8 @@ void main() {
// github publishing
await user.withBrowserPage((page) async {
await page.gotoOrigin('/packages/test_pkg/admin');
await page.takeScreenshots(
prefix: 'package-page/admin-page', selector: 'body');

await page.waitAndClick('#-pkg-admin-automated-github-enabled');
await page.waitForLayout([
Expand Down
4 changes: 4 additions & 0 deletions pkg/pub_integration/test/report_test.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ 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:test/test.dart';

Expand Down Expand Up @@ -58,6 +59,9 @@ void main() {
(page) async {
await page.gotoOrigin('/report?subject=package:oxygen');
await page.waitAndClick('.report-page-direct-report');
await page.takeScreenshots(
prefix: 'report-page/direct-report',
selector: '#report-page-form');
await page.waitFocusAndType('#report-email', 'reporter@pub.dev');
await page.waitFocusAndType(
'#report-message', 'Huston, we have a problem.');
Expand Down
5 changes: 5 additions & 0 deletions pkg/pub_integration/test/search_completition_test.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ 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:test/test.dart';
Expand Down Expand Up @@ -47,6 +48,8 @@ void main() {
await page.gotoOrigin('/');
await page.keyboard.type('is:un');
await Future.delayed(Duration(milliseconds: 200));
await page.takeScreenshots(
selector: 'body', prefix: 'landing-page/search-completion');
await page.keyboard.press(Key.enter);
await Future.delayed(Duration(milliseconds: 200));
await page.keyboard.press(Key.enter);
Expand All @@ -62,6 +65,8 @@ void main() {
// go to the end of the input field and start typing
await page.keyboard.press(Key.arrowDown);
await page.keyboard.type(' -sdk:fl');
await page.takeScreenshots(
selector: 'body', prefix: 'listing-page/search-completion');
await Future.delayed(Duration(milliseconds: 200));
await page.keyboard.press(Key.enter);
await Future.delayed(Duration(milliseconds: 200));
Expand Down
4 changes: 4 additions & 0 deletions pkg/pub_integration/test/search_update_test.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ 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:test/test.dart';
Expand Down Expand Up @@ -99,6 +100,9 @@ void main() {
await _waitOneSecond();
expect(await flutterCB3.boundingBox, isNotNull);

await page.takeScreenshots(
selector: '.search-form', prefix: 'listing-page/search-form');

// click Flutter
await flutterCB3.clickAndWaitOneResponse();
await _waitOneSecond();
Expand Down
Loading