-
Notifications
You must be signed in to change notification settings - Fork 5
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
test: adding e2e tests for the dapp #73
Conversation
- name: yarn test | ||
run: yarn test | ||
- name: yarn test:e2e | ||
run: yarn test:e2e | ||
integration: |
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.
These steps were removed for two reasons:
-
The test in the file
ui/test/App.e2e.ts
were eliminated. The test was basic and focused on checking if the UI was functional, a task already covered by the e2e tests implicitly. -
Including these steps was causing a failure in our CI process. This failure was due to the involvement of tests in this PR defined in the e2e folder. Currently, I am in the process of setting up proper CI for our e2e tests, as detailed in this pull request.
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 just realized... we took out yarn test
?!
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.
LGTM. cc @dckc who's more familiar with the dapp
"eslint-config-turbo": "^1.13.0", | ||
"eslint-plugin-chai-friendly": "^0.7.4", | ||
"eslint-plugin-cypress": "^2.15.1", | ||
"eslint-plugin-testing-library": "^6.2.0", | ||
"eslint-plugin-ui-testing": "^2.0.1" |
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.
these are all needed by @agoric/synpress
? maybe they should be dependencies
so they're pulled in without this additional work and maintenance
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, they're required by @agoric/synpress
.
And I was thinking the same about adding them as dependencies
because it's been quite tedious to add them in every dapp I've worked with so far. Will discuss it with Fraz to confirm as he worked with the linting stuff.
e7d0b07
to
135a05d
Compare
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 struggling to run it locally.
I don't think I'm completely blocked yet, but I'll share my review notes so far...
const updatedPath = directories.join(path.sep); | ||
const synpressPath = path.join(updatedPath, '/node_modules/@agoric/synpress'); | ||
|
||
module.exports = { |
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.
Why use .cjs?
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.
Because in offer-up dApp
we are using ES modules. But @agoric/synpress
uses commonJS modules.
On line 24:
https://github.com/agoric-labs/synpress/blob/dev/package.json
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 makes me curious why synpress uses commonJS. But I guess it's not critical.
const config = require('@agoric/synpress/synpress.config'); | ||
const { defineConfig } = require('cypress'); | ||
|
||
module.exports = defineConfig({ |
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.
why .cjs?
@@ -7,7 +7,7 @@ | |||
"dev": "vite", | |||
"build": "tsc && vite build", | |||
"test": "vitest spec", | |||
"test:e2e": "vitest e2e", | |||
"test:e2e": "EXTENSION=keplr synpress run --configFile=test/e2e/synpress.config.cjs", |
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 wonder if I missed some docs somewhere. I get an error about SECRET_WORDS ...
$ yarn test:e2e
yarn run v1.22.21
$ yarn workspace offer-up-ui test:e2e
$ EXTENSION=keplr synpress run --configFile=test/e2e/synpress.config.cjs
/home/connolly/projects/dapp-offer-up/node_modules/@agoric/synpress/synpress.js:43
throw new Error(
^
Error: Please provide SECRET_WORDS or PRIVATE_KEY environment variable
const alertShown = cy.stub().as('alertShown'); | ||
cy.on('window:alert', alertShown); | ||
|
||
cy.contains('Connect Wallet').click(); |
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 struggling to get it to work.
yarn test:e2e results
$ SECRET_WORDS="powder bicycle deputy will move hen romance pear spring chest yard hunt cruise lumber host plug list trend alone frame sick define february copper" yarn test:e2e
yarn run v1.22.21
$ yarn workspace offer-up-ui test:e2e
$ EXTENSION=keplr synpress run --configFile=test/e2e/synpress.config.cjs
It looks like this is your first time using Cypress: 12.17.3
✔ Verified Cypress! /home/connolly/.cache/Cypress/12.17.3/Cypress
Opening Cypress...
=============================================================================
(Run Starting)
┌────────────────────────────────────────────────────────────────────────────────────────────────┐
│ Cypress: 12.17.3 │
│ Browser: Chrome 123 │
│ Node Version: v20.11.1 (/home/connolly/.nvm/versions/node/v20.11.1/bin/node) │
│ Specs: 1 found (test.spec.js) │
│ Searched: test/e2e/specs/**/*spec.{js,jsx,ts,tsx} │
└────────────────────────────────────────────────────────────────────────────────────────────────┘
────────────────────────────────────────────────────────────────────────────────────────────────────
Running: test.spec.js (1 of 1)
DAPP Offer Up E2E Test Cases
Test commands
✓ should complete Keplr setup by importing an existing wallet using 24 word phrase (28048ms)
1) should reject connection with wallet
2) should accept connection with wallet
3) should reject make an offer transaction
4) should confirm make an offer transaction
1 passing (2m)
4 failing
1) DAPP Offer Up E2E Test Cases
Test commands
should reject connection with wallet:
AssertionError: Timed out retrying after 30000ms: Expected to find content: 'Connect Wallet' but never did.
at Context.eval (webpack:///./specs/test.spec.js:15:9)
2) DAPP Offer Up E2E Test Cases
Test commands
should accept connection with wallet:
AssertionError: Timed out retrying after 30000ms: Expected to find content: 'Connect Wallet' but never did.
at Context.eval (webpack:///./specs/test.spec.js:25:9)
3) DAPP Offer Up E2E Test Cases
Test commands
should reject make an offer transaction:
AssertionError: Timed out retrying after 30000ms: Expected to find content: 'Make an Offer' but never did.
at Context.eval (webpack:///./specs/test.spec.js:34:9)
4) DAPP Offer Up E2E Test Cases
Test commands
should confirm make an offer transaction:
AssertionError: Timed out retrying after 30000ms: Expected to find content: 'Make an Offer' but never did.
at Context.eval (webpack:///./specs/test.spec.js:48:9)
(Results)
┌────────────────────────────────────────────────────────────────────────────────────────────────┐
│ Tests: 5 │
│ Passing: 1 │
│ Failing: 4 │
│ Pending: 0 │
│ Skipped: 0 │
│ Screenshots: 4 │
│ Video: true │
│ Duration: 2 minutes, 28 seconds │
│ Spec Ran: test.spec.js │
└────────────────────────────────────────────────────────────────────────────────────────────────┘
(Screenshots)
- /home/connolly/projects/dapp-offer-up/ui/test/e2e/screenshots/test.spec.js/DAPP (945x848)
Offer Up E2E Test Cases -- Test commands -- should reject connection with wallet
(failed).png
- /home/connolly/projects/dapp-offer-up/ui/test/e2e/screenshots/test.spec.js/DAPP (945x848)
Offer Up E2E Test Cases -- Test commands -- should accept connection with wallet
(failed).png
- /home/connolly/projects/dapp-offer-up/ui/test/e2e/screenshots/test.spec.js/DAPP (945x848)
Offer Up E2E Test Cases -- Test commands -- should reject make an offer transact
ion (failed).png
- /home/connolly/projects/dapp-offer-up/ui/test/e2e/screenshots/test.spec.js/DAPP (945x848)
Offer Up E2E Test Cases -- Test commands -- should confirm make an offer transac
tion (failed).png
(Video)
- Started compressing: Compressing to 32 CRF
- Finished compressing: 10 seconds
Compression progress: 100%
- Video output: /home/connolly/projects/dapp-offer-up/ui/test/e2e/videos/test.spec.js.mp4
=============================================================================
(Run Finished)
Spec Tests Passing Failing Pending Skipped
┌────────────────────────────────────────────────────────────────────────────────────────────────┐
│ ✖ test.spec.js 02:28 5 1 4 - - │
└────────────────────────────────────────────────────────────────────────────────────────────────┘
✖ 1 of 1 failed (100%) 02:28 5 1 4 - -
Cypress run results: 5 total tests, 1 passed, 4 failed
error Command failed with exit code 4.
info Visit https://yarnpkg.com/en/docs/cli/run for documentation about this command.
error Command failed.
Exit code: 4
Command: /home/connolly/.nvm/versions/node/v20.11.1/bin/node
Arguments: /home/connolly/.cache/node/corepack/yarn/1.22.21/lib/cli.js test:e2e
Directory: /home/connolly/projects/dapp-offer-up/ui
Output:
info Visit https://yarnpkg.com/en/docs/cli/workspace for documentation about this command.
error Command failed with exit code 4.
info Visit https://yarnpkg.com/en/docs/cli/run for documentation about this command.
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 tried again with the right mnemonic:
SECRET_WORDS="spike siege world rather ordinary upper napkin voice brush oppose junior route trim crush expire angry seminar anchor panther piano image pepper chest alone" yarn test:e2e
but the address shown in the test is agoric1p2aqakv3ulz4qfy2nut86j9gx0dx0yw09h96md
. I don't know where that's coming from.
The "spike siege..." words correspond to agoric1rwwley550k9mmk6uq6mm6z4udrg8kyuyvfszjk
.
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.
@dckc This is actually a bug in @agoric/synpress
.
At the moment, we hardcode the mnemonic:
'orbit bench unit task food shock brand bracket domain regular warfare company announce wheel grape trust sphere boy doctor half guard ritual three ecology',
Adding an issue for it in @agoric/synpress
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 added it as an over here.
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 struggling to get it to work.
It could be something I'm doing wrong, but I think at least 1 reviewer should be able to run it locally.
@dckc My mistake. I should've been detailed and not assumed things.
|
it(`should complete Keplr setup by importing an existing wallet using 24 word phrase`, () => { | ||
cy.setupWallet().then((setupFinished) => { |
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.
the import-24-words stuff is happening twice when I run the test. Is that by design?
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.
@dckc Yes it is by design. But it's something we are planning to remove in the next release of @agoric/synpress
.
I don't see that src in the keyring. And that amt is kinda huge. But I sorta fumbled my way thru funding the ...md account. |
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 look forward to the lights-out ci version.
Meanwhile, once I fumbled my way thru the account setup, I can run it:
Running: test.spec.js (1 of 1)
DAPP Offer Up E2E Test Cases
Test commands
✓ should complete Keplr setup by importing an existing wallet using 24 word phrase (22848ms)
✓ should reject connection with wallet (616ms)
✓ should accept connection with wallet (1571ms)
✓ should reject make an offer transaction (2595ms)
✓ should confirm make an offer transaction (5554ms)
5 passing (33s)
The PR introduces end-to-end (e2e) tests for the Offer-Up decentralized application (dApp).
Currently, these tests are executable locally. To get started,
orbit bench unit...ecology
that corresponds toagoric1p2aqakv3ulz4qfy2nut86j9gx0dx0yw09h96md
for testing purposes. To ensure this wallet has enough ISTs to make transactions, you need todocker exec
the local chain and run this script:In an upcoming PR, I plan to add support for running these tests in our CI pipeline.