feat(screenshot): Add screenshot function to e2e test (button and checkbox) #2532

tinayuangao · 2017-01-05T00:35:51Z

jelbourn

Did you determine whether writing / reading these files works on SauceLabs?

jelbourn · 2017-01-05T00:44:22Z

e2e/screenshot.ts

+  get filename() {
+    return this.id
+        .toLowerCase()
+        .replace(/[ :\/]/g, '_')


Can change the space to \s to capture all whitespace.

jelbourn · 2017-01-05T00:45:41Z

e2e/screenshot.ts

+   * The filename used to store the screenshot
+   * @returns {string}
+   */
+  get filename() {


Can do get filename(): string and remove the type from the JsDoc
(here and elsewhere for return types and params)

jelbourn · 2017-01-05T00:46:07Z

e2e/screenshot.ts

+   * The full path to the screenshot
+   * @returns {string}
+   */
+  get path() {


Let's call this fullPath

jelbourn · 2017-01-05T00:55:36Z

e2e/components/button/button.e2e.ts

      browser.get('/button');
    });
    it('should prevent click handlers from executing when disabled', function () {
+      let testname =


Jasmine 2.0 made it more difficult to get the current test name, but it's still possible by adding a custom reporter. Example.

Could could add a reporter that just updates a value which is then read in screenshot.ts

Added a reporter and we can use the test name in Screenshot now

jelbourn · 2017-01-05T00:57:54Z

e2e/screenshot.ts

+    if (!fs.existsSync(dir)) {
+      fs.mkdirSync(dir, 0o744);
+    }
+    fs.writeFileSync(this.path, png, {encoding: 'base64'});


Why base64 encoding? Shouldn't this be binary?

The output of binary cannot be opened as an image.

jelbourn · 2017-01-05T00:58:52Z

e2e/screenshot.ts

+  storeScreenshot(png: any) {
+    var dir = path.resolve(__dirname, '..', 'screenshots');
+    if (!fs.existsSync(dir)) {
+      fs.mkdirSync(dir, 0o744);


Why octal instead of hexadecimal?

tinayuangao · 2017-01-06T22:34:59Z

Fixed test on travis-ci and sauce labs. Please take another look. Thanks!

jelbourn · 2017-01-06T23:58:08Z

e2e/screenshot.ts

+
+export class Screenshot {
+  id: string;
+  dir: string = '/tmp/angular-material2-build/screenshots/';


Make this a file-level constant, e.g.,

/** Directory into which screenshots will be written. */ const OUTPUT_DIR = '/tmp/angular-material2-build/screenshots/';

jelbourn · 2017-01-07T00:01:43Z

e2e/screenshot.ts

+function initializeEnvironment(jasmine: any) {
+  var reporter = new jasmine.JsApiReporter({});
+  reporter.specStarted = function(result: any) {
+    jasmine.getEnv().currentSpec = result.fullName;


Instead of storing this to jasmine.getEnv(), would it work to create a variable at the file level and update that? E.g.,

let currentJasmineSpecName = ''; /** Adds a custom jasmine reporter that simply keeps track of the current test name. */ function installJasmineReporter(jasmine: any) { jasmine.getEnv().addReporter(new jasmine.JsApiReporter({ specStarted: (result: any) => { currentJasmineSpecName = result.fullName; } })); }

I also just realized: we should make sure that the test results are still being reported normally with the custom report installed

Changed to file-level variable.

Constructor JsApiReporter({specStarted}) is not working.

Tested the test results are being reported normally.

jelbourn

LGTM

devversion · 2017-01-13T23:37:00Z

@jelbourn @tinayuangao I just have some questions:

How would you look at the screenshots in the CI?
Is this the base for screenshot-diffs or is it just to look at errors in E2E tests?

(Asking because Saucelabs can already do screenshots)

angular-automatic-lock-bot · 2019-09-06T07:01:28Z

This issue has been automatically locked due to inactivity.
Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

_{This action has been performed automatically by a bot.}

googlebot added the cla: yes PR author has agreed to Google's Contributor License Agreement label Jan 5, 2017

tinayuangao requested a review from jelbourn January 5, 2017 00:36

jelbourn reviewed Jan 5, 2017

View reviewed changes

Add screenshot function to e2e test (button and checkbox)

b40999b

tinayuangao force-pushed the screenshot-pr branch 6 times, most recently from eb60e39 to 2b4e66d Compare January 6, 2017 22:02

Add reporter for jasmine

a48c356

tinayuangao force-pushed the screenshot-pr branch from 6b45e60 to a48c356 Compare January 6, 2017 22:24

tinayuangao changed the title ~~Add screenshot function to e2e test (button and checkbox)~~ feat(screenshot): Add screenshot function to e2e test (button and checkbox) Jan 6, 2017

jelbourn reviewed Jan 7, 2017

View reviewed changes

tinayuangao force-pushed the screenshot-pr branch 2 times, most recently from 53135ce to 1670ee4 Compare January 7, 2017 00:48

Change directory to constant, and use variable to store name

268c55a

tinayuangao force-pushed the screenshot-pr branch from c312aa6 to 268c55a Compare January 7, 2017 00:51

jelbourn approved these changes Jan 11, 2017

View reviewed changes

jelbourn added pr: lgtm action: merge The PR is ready for merge by the caretaker labels Jan 11, 2017

tinayuangao merged commit 8ba8deb into angular:master Jan 12, 2017

angular-automatic-lock-bot bot locked and limited conversation to collaborators Sep 6, 2019

feat(screenshot): Add screenshot function to e2e test (button and checkbox) #2532

feat(screenshot): Add screenshot function to e2e test (button and checkbox) #2532

Uh oh!

Conversation

tinayuangao commented Jan 5, 2017

Uh oh!

jelbourn left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

tinayuangao commented Jan 6, 2017

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jelbourn Jan 7, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jelbourn left a comment

Choose a reason for hiding this comment

Uh oh!

devversion commented Jan 13, 2017

Uh oh!

angular-automatic-lock-bot bot commented Sep 6, 2019

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

jelbourn Jan 7, 2017 •

edited

Loading