* Don't use expect within Promises (#5466)
If a call to expect fails within a Promise it will not
be resolved, and causing the test to crash.
The patch aligns the code similar to what is used by all
the other tests.
When this test was failing, it would cause no future tests to run. This
was because the `expect` call within the `page.on` callback would throw
an error, and that would trigger a unhandled promise rejection that
caused the test framework to stop.
The fundamental issue here is making `expect` calls within callbacks.
They are brittle due to the fact that they throw, and the test framework
won't catch it, but also because you have no guarantee that they will
run. If the callback is never executed you dont' know about it.
Although it's slightly more code, using a stub is the way to do this.
Not only can we assert that the stub was called, we can make synchronous
`expect` calls that Mocha will pick up properly if they fail.
Before this change, running the tests (and making it fail on purpose)
would cause all test execution to stop:
```
> puppeteer@3.0.4-post unit /Users/jacktfranklin/src/puppeteer
> mocha --config mocha-config/puppeteer-unit-tests.js
.(node:69580) UnhandledPromiseRejectionWarning: Error: expect(received).toBe(expected) // Object.is equality
Expected: "yes."
Received: ""
at Page.<anonymous> (/Users/jacktfranklin/src/puppeteer/test/dialog.spec.js:42:37)
[snip]
(node:69580) UnhandledPromiseRejectionWarning: Unhandled promise rejection ... [snip]
```
But with this change, the rest of the tests run:
```
> puppeteer@3.0.4-post unit /Users/jacktfranklin/src/puppeteer
> mocha --config mocha-config/puppeteer-unit-tests.js
Page.Events.Dialog
✓ should fire
1) should allow accepting prompts
✓ should dismiss the prompt
2 passing (2s)
1 failing
1) Page.Events.Dialog
should allow accepting prompts:
Error: expect(received).toBe(expected) // Object.is equality
Expected: "yes."
Received: ""
at Context.<anonymous> (test/dialog.spec.js:53:35)
at processTicksAndRejections (internal/process/task_queues.js:94:5)
```
This is much better because one failing test now doesn't stop the rest
of the test suite.
This probably isn't the only instance of this in the codebase so I
propose as we encounter them we fix them usng this commit as the
template.
The codebase was incredibly inconsistent with the use of spacing around
curly braces, e.g.:
```
// this?
const a = {b: 1}
// or?
const a = { b: 1 }
```
This extended into import statements also. Google's styleguide is no
spacing, so we're going with that.
Rather than maintain our own test runner we should instead lean on the community and use Mocha which is very popular and also our test runner of choice in DevTools too.
Note that this commit doesn't remove the TestRunner source as it's still used for other unit tests, but they will be updated in a future PR and then we can remove the TestRunner.
The main bulk of this PR is updating the tests as the old TestRunner passed in contextual data via the `it` function callback whereas Mocha does not, so we introduce some helpers for the tests to make it easier.
