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

SCP-3452 chain-index e2e tests #312

Merged
merged 2 commits into from
Feb 22, 2022

Conversation

catch-21
Copy link
Contributor

Rest API tests that hit each chain-index endpoint. Designed to run on testnet, should be easy enough to extend for mainnet too if desired. Can look at extending this for more complex scenarios such as involving live transactions to check behaviour and performance.

Pre-submit checklist:

  • Branch
    • Tests are provided (if possible)
    • Commit sequence broadly makes sense
    • Key commits have useful messages
    • Relevant tickets are mentioned in commit messages
    • Formatting, materialized Nix files, PNG optimization, etc. are updated
  • PR
    • Self-reviewed the diff
    • Useful pull request description
    • Reviewer requested

@catch-21 catch-21 marked this pull request as draft February 10, 2022 15:31
@catch-21
Copy link
Contributor Author

catch-21 commented Feb 10, 2022

Will open for review after I fix Typescript transpiling

@catch-21 catch-21 force-pushed the chain-index-e2e-tests branch 2 times, most recently from 2c537ee to 9b16666 Compare February 15, 2022 16:54
@catch-21 catch-21 marked this pull request as ready for review February 15, 2022 16:56
Copy link
Contributor

@raduom raduom left a comment

Choose a reason for hiding this comment

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

LGTM

@catch-21 catch-21 force-pushed the chain-index-e2e-tests branch 2 times, most recently from a5b10e1 to 569a261 Compare February 16, 2022 15:32
- Update URL in environment file if needed

### Run tests:
- `nvm use`
Copy link
Contributor

Choose a reason for hiding this comment

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

BTW, I think this is optional for users of nix-shell as node is already available (currently v14.17.4).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sure. I could remove this cmd and nvmrc file and we always assume run in nix-shell..?

Copy link
Contributor

Choose a reason for hiding this comment

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

Yeah exactly! However, you can leave this for those people not using nix-shell, and say that it's unnecessary for those using nix-shell

@catch-21 catch-21 merged commit da8142c into IntersectMBO:main Feb 22, 2022
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.

4 participants