Skip to content
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 tests for setting up governance params for liquidation flow and ci changes #156

Closed

Conversation

rabi-siddique
Copy link
Contributor

@rabi-siddique rabi-siddique commented Apr 18, 2024

The PR does the following:

  • Add tests for setting up manager and auctioneer params for Liquidation.
  • Update CI configurations to execute these tests.

This is the first PR for adding tests for implementing Happy Path Scanerio Liquidation. It will be followed by subsequent PRs containing other tests.

@rabi-siddique rabi-siddique force-pushed the rs-setting-governance-params branch 3 times, most recently from f1796af to 075a93c Compare April 18, 2024 15:20
@rabi-siddique rabi-siddique changed the title Rs setting governance params test: adding tests for setting up governance params for liquidation flow and ci changes Apr 18, 2024
@rabi-siddique rabi-siddique force-pushed the rs-setting-governance-params branch from 075a93c to 060bede Compare April 18, 2024 15:33
@@ -1 +1 @@
import '@agoric/synpress/support/index';
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

reason for changing this? index should work just fine

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have customized the support.js file in @agoric/synpress. In the before hook, we originally set up the wallet. That sets the wallet every time I visit a different origin. I found this approach to be excessive, so I opted to override it. Now, I configure the wallet setup explicitly within a specific test to ensure we don't set the wallet everytime we visit a different origin.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was wondering if we should remove setting up wallet in the before hook?

@@ -1,7 +1,8 @@
/* eslint-disable ui-testing/no-disabled-tests */
describe('Wallet App Test Cases', () => {
context('Test commands', () => {
it(`should connect with Agoric Chain`, () => {
it(`should setup wallet and connect with Agoric Chain`, () => {
cy.setupWallet();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we dont have the SKIP_EXTENSION_SETUP flag enabled anywhere, wont this duplicate the setup of the wallet?

@rabi-siddique rabi-siddique linked an issue Apr 19, 2024 that may be closed by this pull request
entrypoint: []
working_dir: /app
volumes:
- ./cypress/videos:/app/test/e2e/videos
- ./cypress/screenshots:/app/test/e2e/screenshots
command: >
bash -c 'echo -n "======> local noVNC URL: http://localhost:8080/vnc.html?autoconnect=true " && yarn wait-on http://display:8080 && echo -n "======> remote noVNC URL: " && curl -s ngrok:4040/api/tunnels | jq -r .tunnels[0].public_url && yarn test:e2e:ci'
bash -c 'echo -n "======> local noVNC URL: http://localhost:8080/vnc.html?autoconnect=true " && yarn wait-on http://display:8080 && echo -n "======> remote noVNC URL: " && curl -s ngrok:4040/api/tunnels | jq -r .tunnels[0].public_url && nginx && yarn test:e2e:ci'
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This long line makes it hard to see changes. Consider a script with new lines

it('should set up wallets for two members of the econ committee.', () => {
cy.setupWallet({
secretWords:
'such field health riot cost kitten silly tube flash wrap festival portion imitate this make question host bitter puppy wait area glide soldier knee',
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Magic string. Please source from constants in a config module


context('Adjusting Vault Params', () => {
it('should allow gov1 to create a proposal', () => {
cy.visit('https://econ-gov.inter.trade/?agoricNet=local');
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Consider a const

@rabi-siddique rabi-siddique linked an issue Apr 22, 2024 that may be closed by this pull request
@rabi-siddique rabi-siddique marked this pull request as draft May 17, 2024 16:50
@rabi-siddique rabi-siddique removed the request for review from samsiegart May 17, 2024 16:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Liquidation Testing Completed
3 participants