-
-
Notifications
You must be signed in to change notification settings - Fork 1.5k
test(client): check console.log #1907
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
Conversation
setTimeout(() => { | ||
expect(res).toMatchSnapshot(); | ||
browser.close().then(resolve); | ||
}, 3000); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe here is flaky tests.
Codecov Report
@@ Coverage Diff @@
## master #1907 +/- ##
=======================================
Coverage 87.96% 87.96%
=======================================
Files 15 15
Lines 814 814
Branches 259 259
=======================================
Hits 716 716
Misses 84 84
Partials 14 14 Continue to review full report at Codecov.
|
exports[`Client console.log hot disabled 1`] = ` | ||
Array [ | ||
"Hey.", | ||
"[WDS] Live Reloading enabled.", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should it be output this line?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes
|
||
exports[`Client console.log liveReload enabled 1`] = ` | ||
Array [ | ||
"Hey.", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should expect [WDS] Live Reloading enabled.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, need investigate what is problem
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's looks good.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good, but will be great to write tests without running real browser, it is very slow test and looks like integration than unit
|
||
exports[`Client console.log liveReload enabled 1`] = ` | ||
Array [ | ||
"Hey.", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, need investigate what is problem
client-src depends on browsers variables, e.g |
@hiroppy jest support jsdom and windows and other tools avaliable, but maybe it is bad idea try to use jsdom because many features are unavailable, so i think it is ok using this approach |
Anyway need investigate why some value invalid in tests as i written above |
Yeah, I agree with you.
I would like to investigate after merging this PR. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Don't forget to investigate why tests invalid in some cases
aa0d9fb
cac171b
to
aa0d9fb
Compare
I just fixed lint. PTAL /cc @evilebottnawi |
And I'll start to investigate client code, so I need this pr. |
For Bugs and Features; did you add new tests?
yes
Motivation / Use-Case
Just testing
console.log
of the client's code.Breaking Changes
no
Additional Info
Maybe liveReload has a bug... see the snapshot.