-
Notifications
You must be signed in to change notification settings - Fork 58
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
Add e2e smoke test #156
Add e2e smoke test #156
Conversation
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.
Looking great!
@olizilla do you think there's anything we can do to circle.ci? Otherwise, should we just remove it? |
@pgte I don't think we need both, I'd like to remove circle for now, and just use travis until jenkins is ready for all the responsibilities. |
README.md
Outdated
@@ -6,7 +6,7 @@ | |||
<a href="https://protocol.io"><img src="https://img.shields.io/badge/made%20by-Protocol%20Labs-blue.svg?style=flat-square" /></a> | |||
<a href="http://peerpad.net/"><img src="https://img.shields.io/badge/project-PeerPad-blue.svg?style=flat-square" /></a> | |||
<a href="http://webchat.freenode.net/?channels=%23ipfs"><img src="https://img.shields.io/badge/freenode-%23ipfs-blue.svg?style=flat-square" /></a> | |||
<a href="https://travis-ci.org/ipfs-shipyard/peerpad"><img src="https://img.shields.io/travis/ipfs-shipyard/peerpad/master.svg?style=flat-square" /></a> | |||
[![Build Status](https://travis-ci.org/ipfs-shipyard/peer-pad.svg?branch=master)](https://travis-ci.org/ipfs-shipyard/peer-pad) |
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.
Markdown syntax won't render within HTML
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.
maaan. It renders in atom's markdown preview!
|
||
```bash | ||
URL=https://peerpad.net npm run test:e2e | ||
``` |
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.
✨
## Deploy | ||
|
||
You can self-host your own Peerpad. For that, deploy the `build` directory (after you have run the `npm run build` command). | ||
You can self-host your own Peerpad. For that, run `npm run build` and deploy the `build` directory to a web-server |
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.
...or run test:serve
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.
I'm not gonna recommend people use http-server
in prod, even if it does think it's production ready.
test/e2e/smoke.test.js
Outdated
} | ||
|
||
async function findPeerId (page) { | ||
// wait up to 1 minute for ifps to boot |
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.
typo "ifps"
test/e2e/smoke.test.js
Outdated
expect(bobPeerId).not.toEqual(alfPeerId) | ||
console.log('alf peerId', alfPeerId) | ||
console.log('bob peerId', bobPeerId) | ||
// wait for ifps to sync... |
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.
typo "ifps"
test/e2e/smoke.test.js
Outdated
browsers.forEach(b => b.close()) | ||
}) | ||
|
||
it('Create a pad', async (done) => { |
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.
You don't need to async
and done
. If using async
and something fails, just throw.
https://facebook.github.io/jest/docs/en/asynchronous.html#async-await
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.
Good spot. I was getting unhandedRejectedPromise
warnings rather than test failures, which is why these tests are a weird mixture of async await
and done
callbacks. I need to dig into that.
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.
I've tried to recreate the madness and didn't get the same issue so have fixed up the tests to be more sane as per your suggests. much nice.
test/e2e/smoke.test.js
Outdated
done.fail(err) | ||
} | ||
done() | ||
}, waitFor) |
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.
To avoid having to use done
in this test due to callbacks you could:
await pause(waitFor)
const bobTitle = await getDocumentTitle(bob)
expect(bobTitle).toEqual(alfTitle)
function pause (ms) {
return new Promise(resolve => setTimeout(resolve, ms))
}
test/e2e/smoke.test.js
Outdated
await page.goto(appUrl) | ||
await page.waitForSelector('[data-id=start-button]') | ||
await page.click('[data-id=start-button]') | ||
// wait up to 1 minute for ifps to boot |
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.
typo "ifps"
To try and reduce the chance of another #150 event, this PR adds an end to end smoke test that'll
Uses puppeteer for the headless chrome magic.
It runs in travis, so this should prevent totally broken things getting merged. I don't plan to add lot's more full integration tests like this; this is the test of last resort. The next step is to clean up the code base and add a bunch of unit tests.