fix(e2e): screenshot changes will display proper error message #194
Conversation
googlebot
added
the
cla: yes
| this.overwriteExistingScreenshot(); | ||
| if (referenceScreenshot.compare(this.png)) { | ||
| throw new Error(`screenshot "${this.id}" has changed.`); | ||
| throw `screenshot "${this.id}" has changed.`; |
There was a problem hiding this comment.
Why throw a string instead of an error? Add a comment?
|
Can address commit in a follow-up if necessary. LGTM for getting the build green. |
|
@jelbourn No real reason, to be honest, other than when I was looking at what others were doing in Node, it seemed more common to just throw a string. |
| it('should have a title', function () { | ||
| expect(browser.getTitle()).toBe('Material2'); | ||
| screenshot('initial state'); | ||
| screenshot('demo page: index'); |
There was a problem hiding this comment.
Is that still going in the file name? If so Windows is allergic to :s
There was a problem hiding this comment.
Ah, good point. I plan to add filtering to that so it will strip special
characters. Ideally, I think it would be nice to use slashes... like
"component_name/screenshot_name".
On Sat, Mar 19, 2016 at 9:09 PM, Hans [email protected] wrote:
In e2e/index.e2e.js
#194 (comment):@@ -5,7 +5,7 @@ describe('hello, protractor', function () {
browser.get('/');
it('should have a title', function () {
expect(browser.getTitle()).toBe('Material2');
screenshot('initial state');screenshot('demo page: index');Is that still going in the file name? If so Windows is allergic to :s
—
You are receiving this because you authored the thread.
Reply to this email directly or view it on GitHub
https://github.com/angular/material2/pull/194/files/d34400bbbf1171b866c75802d269e0fd830c6c93#r56759019
There was a problem hiding this comment.
Then do that in this PR if you're going to change the name :)
On Mar 19, 2016, 21:12 -0700, Robert [email protected], wrote:
Ine2e/index.e2e.js(#194 (comment)):
@@ -5,7 +5,7 @@ describe('hello, protractor', function () {>browser.get('/');>it('should have a title', function () {>expect(browser.getTitle()).toBe('Material2');>- screenshot('initial state');>+ screenshot('demo page: index');
Ah, good point. I plan to add filtering to that so it will strip special characters. Ideally, I think it would be nice to use slashes... like "component_name/screenshot_name".
…(#)
On Sat, Mar 19, 2016 at 9:09 PM, [email protected]: In e2e/index.e2e.js<#194 (comment)(https://github.com/angular/material2/pull/194#discussion_r56759019)>:>@@ -5,7 +5,7 @@ describe('hello, protractor', function () {>browser.get('/');>it('should have a title', function () {>expect(browser.getTitle()).toBe('Material2');>- screenshot('initial state');>+ screenshot('demo page: index'); Is that still going in the file name? If so Windows is allergic to :s — You are receiving this because you authored the thread. Reply to this email directly or view it on GitHubhttps://github.com//pull/194/files/d34400bbbf1171b866c75802d269e0fd830c6c93#r56759019—
You are receiving this because you were mentioned.
Reply to this email directly orview it on GitHub(https://github.com/angular/material2/pull/194/files/d34400bbbf1171b866c75802d269e0fd830c6c93#r56759038)
There was a problem hiding this comment.
Yea, I'll update that on Monday.
On Sat, Mar 19, 2016 at 9:13 PM, Hans [email protected] wrote:
In e2e/index.e2e.js
#194 (comment):@@ -5,7 +5,7 @@ describe('hello, protractor', function () {
browser.get('/');
it('should have a title', function () {
expect(browser.getTitle()).toBe('Material2');
screenshot('initial state');screenshot('demo page: index');Then do that in this PR if you're going to change the name :)
… <#-7721814002937843640_>
On Mar 19, 2016, 21:12 -0700, Robert [email protected],
wrote: Ine2e/index.e2e.js(#194 (comment)
#194 (comment)):@@ -5,7 +5,7 @@ describe('hello, protractor', function ()
{>browser.get('/');>it('should have a title', function ()
{>expect(browser.getTitle()).toBe('Material2');>- screenshot('initial
state');>+ screenshot('demo page: index'); Ah, good point. I plan to add
filtering to that so it will strip special characters. Ideally, I think it
would be nice to use slashes... like "component_name/screenshot_name". …(#)
On Sat, Mar 19, 2016 at 9:09 PM, [email protected]: In
e2e/index.e2e.js<#194 #194
(comment)(#194 (comment)
#194 (comment) -5,7
+5,7 @@ describe('hello, protractor', function ()
{>browser.get('/');>it('should have a title', function ()
{>expect(browser.getTitle()).toBe('Material2');>- screenshot('initial
state');>+ screenshot('demo page: index'); Is that still going in the file
name? If so Windows is allergic to :s — You are receiving this because you
authored the thread. Reply to this email directly or view it on GitHub<
https://github.com/angular/material2/pull/194/files/d34400bbbf1171b866c75802d269e0fd830c6c93#r56759019>
— You are receiving this because you were mentioned. Reply to this email
directly orview it on GitHub(
https://github.com/angular/material2/pull/194/files/d34400bbbf1171b866c75802d269e0fd830c6c93#r56759038)—
You are receiving this because you authored the thread.
Reply to this email directly or view it on GitHub
https://github.com/angular/material2/pull/194/files/d34400bbbf1171b866c75802d269e0fd830c6c93#r56759049
|
This issue has been automatically locked due to inactivity. Read more about our automatic conversation locking policy. This action has been performed automatically by a bot. |
4 participants
closes #191
closes #192