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

Add iota-indexer RPC tests to CI #3781

Conversation

samuel-rufi
Copy link
Member

@samuel-rufi samuel-rufi commented Oct 30, 2024

Description of change

Adds iota-indexer RPC tests to the CI.

Links to any relevant issues

Fixes #3590

Type of change

  • Enhancement (a non-breaking change which adds functionality)

How the change has been tested

I locally ran the required cargo test --profile simulator --package iota-indexer --test rpc-tests --features shared_test_runtime command.

Change checklist

  • I have followed the contribution guidelines for this project
  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • I have added tests that prove my fix is effective or that my feature works
  • I have checked that new and existing unit tests pass locally with my changes

@samuel-rufi samuel-rufi added the infrastructure Issues related to the Infrastructure Team label Oct 30, 2024
@samuel-rufi samuel-rufi self-assigned this Oct 30, 2024
Copy link
Contributor

This pull request has been deployed to Vercel.

Latest commit: 99a5e8b

✅ Preview: https://apps-ui-nrmj9482z-iota1.vercel.app

Copy link
Contributor

This pull request has been deployed to Vercel.

Latest commit: 99a5e8b

✅ Preview: https://apps-backend-29a719c9ccbdb9b80c97b599a599d8953354da-e7fg9a5xc.vercel.app

Copy link
Contributor

This pull request has been deployed to Vercel.

Latest commit: 99a5e8b

✅ Preview: https://iota-rebased-explorer-092603f1de8088243ccb8b861b23e-e7euxuv0s.vercel.app

Copy link
Contributor

This pull request has been deployed to Vercel.

Latest commit: 99a5e8b

✅ Preview: https://wallet-dashboard-a225fcfa1e2f852205a61a7cd1d5d93e92-ewzhqoi77.vercel.app

@@ -244,3 +244,4 @@ jobs:
cargo nextest run --no-fail-fast --test-threads 8 --package iota-graphql-e2e-tests --features pg_integration
cargo nextest run --no-fail-fast --test-threads 1 --package iota-cluster-test --test local_cluster_test --features pg_integration
cargo nextest run --no-fail-fast --test-threads 1 --package iota-indexer --test ingestion_tests --features pg_integration
cargo test --package iota-indexer --features shared_test_runtime
Copy link
Member

Choose a reason for hiding this comment

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

Is that not already covered by another job that would run all the tests?

Copy link
Member Author

Choose a reason for hiding this comment

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

Hmm I couldn't find any relevant cargo test reference. Maybe @DaughterOfMars knows?

Copy link
Contributor

Choose a reason for hiding this comment

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

That's true, it should be

Copy link
Member Author

@samuel-rufi samuel-rufi Oct 30, 2024

Choose a reason for hiding this comment

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

You mean this one?

run: cargo test

Couldn't find anything else.

Copy link
Contributor

Choose a reason for hiding this comment

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

yeah because it runs with all features

Copy link
Member

@thibault-martinez thibault-martinez Oct 31, 2024

Choose a reason for hiding this comment

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

yeah because it runs with all features

Not sure what you meant by this.
Anyway, what I initially wanted to say is that the portion of test without the shared_test_runtime feature is now tested twice, in the usual Rust tests and in this new run.

Copy link
Contributor

Choose a reason for hiding this comment

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

Is it this job that should run with all the features: https://github.com/iotaledger/iota/actions/runs/11597827866/job/32292483790?pr=3781 ?

As far as I see only indexer tests with no features are run there. Ingestion and rpc-tests are skipped.
Are you sure it works that way @DaughterOfMars ?

Copy link
Member Author

@samuel-rufi samuel-rufi Oct 31, 2024

Choose a reason for hiding this comment

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

First of all we need a running postgres instance for this run. Also the coverage run doesn't seem to run with all features as there is no feature flag specified. To avoid duplicate tests in this new run we can just specify/run the RPC tests with:

cargo test --profile simulator --package iota-indexer --features shared_test_runtime --test rpc-tests

Copy link
Member

@thibault-martinez thibault-martinez Oct 31, 2024

Choose a reason for hiding this comment

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

I was not speaking myself about the coverage job, just the usual rust tests job. And yeah I guess we would get rid of the duplicates by specifying which tests we want to run and not just the feature.

Copy link
Member Author

Choose a reason for hiding this comment

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

Fixed with 84bdce1

Copy link
Contributor

This pull request has been deployed to Vercel.

Latest commit: 3524d1f

✅ Preview: https://apps-ui-m46mrrxl4-iota1.vercel.app

Copy link
Contributor

This pull request has been deployed to Vercel.

Latest commit: 3524d1f

✅ Preview: https://apps-backend-29a719c9ccbdb9b80c97b599a599d8953354da-ev11yk0cb.vercel.app

Copy link
Contributor

This pull request has been deployed to Vercel.

Latest commit: 3524d1f

✅ Preview: https://wallet-dashboard-a225fcfa1e2f852205a61a7cd1d5d93e92-fknllahel.vercel.app

Copy link
Contributor

This pull request has been deployed to Vercel.

Latest commit: 3524d1f

✅ Preview: https://iota-rebased-explorer-092603f1de8088243ccb8b861b23e-ksipcs174.vercel.app

@sergiupopescu199 sergiupopescu199 self-requested a review October 30, 2024 12:00
Copy link
Contributor

This pull request has been deployed to Vercel.

Latest commit: ea66c0d

✅ Preview: https://apps-ui-r5jeg4tit-iota1.vercel.app

Copy link
Contributor

This pull request has been deployed to Vercel.

Latest commit: ea66c0d

✅ Preview: https://apps-backend-29a719c9ccbdb9b80c97b599a599d8953354da-jjzoiwb2x.vercel.app

Copy link
Contributor

This pull request has been deployed to Vercel.

Latest commit: ea66c0d

✅ Preview: https://iota-rebased-explorer-092603f1de8088243ccb8b861b23e-5wnlsbt1a.vercel.app

Copy link
Contributor

This pull request has been deployed to Vercel.

Latest commit: ea66c0d

✅ Preview: https://wallet-dashboard-a225fcfa1e2f852205a61a7cd1d5d93e92-rmxwgqx4s.vercel.app

Copy link
Contributor

This pull request has been deployed to Vercel.

Latest commit: e05b801

✅ Preview: https://apps-ui-d6dh7seni-iota1.vercel.app

Copy link
Contributor

This pull request has been deployed to Vercel.

Latest commit: e05b801

✅ Preview: https://iota-rebased-explorer-092603f1de8088243ccb8b861b23e-at8kf304y.vercel.app

Copy link
Contributor

This pull request has been deployed to Vercel.

Latest commit: e05b801

✅ Preview: https://apps-backend-29a719c9ccbdb9b80c97b599a599d8953354da-qmxtx5adw.vercel.app

Copy link
Contributor

This pull request has been deployed to Vercel.

Latest commit: e05b801

✅ Preview: https://wallet-dashboard-a225fcfa1e2f852205a61a7cd1d5d93e92-qoet5fisi.vercel.app

@samuel-rufi samuel-rufi force-pushed the l1sc/infra/iota-indexer-ci-tests branch from 5a93665 to c7e278e Compare October 31, 2024 08:58
Copy link
Contributor

This pull request has been deployed to Vercel.

Latest commit: 5a93665

✅ Preview: https://apps-ui-j2krl0hhn-iota1.vercel.app

Copy link
Contributor

This pull request has been deployed to Vercel.

Latest commit: 5a93665

✅ Preview: https://iota-rebased-explorer-092603f1de8088243ccb8b861b23e-d7rwmrwi6.vercel.app

Copy link
Contributor

This pull request has been deployed to Vercel.

Latest commit: 5a93665

✅ Preview: https://wallet-dashboard-a225fcfa1e2f852205a61a7cd1d5d93e92-fklrkotas.vercel.app

@samuel-rufi samuel-rufi force-pushed the l1sc/infra/iota-indexer-ci-tests branch from c7e278e to 84bdce1 Compare October 31, 2024 09:00
Copy link
Contributor

This pull request has been deployed to Vercel.

Latest commit: c7e278e

✅ Preview: https://apps-backend-29a719c9ccbdb9b80c97b599a599d8953354da-620c041kf.vercel.app

Copy link
Contributor

This pull request has been deployed to Vercel.

Latest commit: 84bdce1

✅ Preview: https://apps-ui-hchhxnwjj-iota1.vercel.app

Copy link
Contributor

This pull request has been deployed to Vercel.

Latest commit: 84bdce1

✅ Preview: https://iota-rebased-explorer-092603f1de8088243ccb8b861b23e-lswtdi3sh.vercel.app

Copy link
Contributor

This pull request has been deployed to Vercel.

Latest commit: 84bdce1

✅ Preview: https://apps-backend-29a719c9ccbdb9b80c97b599a599d8953354da-exvawk6ie.vercel.app

Copy link
Contributor

This pull request has been deployed to Vercel.

Latest commit: 84bdce1

✅ Preview: https://wallet-dashboard-a225fcfa1e2f852205a61a7cd1d5d93e92-h43qnhrhf.vercel.app

Copy link
Contributor

@sergiupopescu199 sergiupopescu199 left a comment

Choose a reason for hiding this comment

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

lgtm ✨

@samuel-rufi samuel-rufi merged commit b6e4ad1 into sc-platform/indexer-new-rpc-tests Oct 31, 2024
35 of 39 checks passed
@samuel-rufi samuel-rufi deleted the l1sc/infra/iota-indexer-ci-tests branch October 31, 2024 09:31
sergiupopescu199 pushed a commit that referenced this pull request Oct 31, 2024
sergiupopescu199 pushed a commit that referenced this pull request Oct 31, 2024
sergiupopescu199 pushed a commit that referenced this pull request Oct 31, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
infrastructure Issues related to the Infrastructure Team
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants