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.

Sign up for GitHub

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

Jump to bottom

chore: update Starknet SDK test workflows #2618

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from
Draft

chore: update Starknet SDK test workflows #2618

wants to merge 1 commit into from

Conversation

wojciechos
Copy link
Contributor

@wojciechos wojciechos commented Mar 6, 2025 •
edited by hudem1
Loading

Fixes issue #2556

  • starknet-go-tests.yml:
    • Remove hardcoded commit reference in favor of using v0.7.3 tag
    • Remove skipped test 'TestBlockWithReceipts'
  • starknet-js-tests.yml:
    • Update starknet.js version from v6.14.1 to v6.23.1
  • starknet-rs-tests.yml:
    • Update starknet-rs version from v0.12.0 to v0.13.0
    • Remove skipped test 'jsonrpc_get_block_with_receipts'

These updates ensure compatibility with the latest SDK versions and enable previously skipped tests, improving test coverage.

@wojciechos
chore: update Starknet SDK test workflows
27cc4ef 
- ci-cd-pipeline.yml: Update branch trigger to 'wojciechos/update-sdk-tests'
- starknet-go-tests.yml:
  - Remove hardcoded commit reference in favor of using v0.7.3 tag
  - Remove skipped test 'TestBlockWithReceipts'
- starknet-js-tests.yml:
  - Update starknet.js version from v6.14.1 to v6.23.1
- starknet-rs-tests.yml:
  - Update starknet-rs version from v0.12.0 to v0.13.0
  - Remove skipped test 'jsonrpc_get_block_with_receipts'

These updates ensure compatibility with the latest SDK versions and enable
previously skipped tests, improving test coverage.
@codecov Codecov
Copy link

codecov bot commented Mar 6, 2025 •
edited
Loading

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 74.74%. Comparing base (998a15c) to head (27cc4ef).
Report is 4 commits behind head on main.

Additional details and impacted files 
@@            Coverage Diff             @@
##             main    #2618      +/-   ##
==========================================
- Coverage   74.82%   74.74%   -0.08%     
==========================================
  Files         139      139              
  Lines       16732    16732              
==========================================
- Hits        12519    12507      -12     
- Misses       3381     3390       +9     
- Partials      832      835       +3

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@rodrigo-pino rodrigo-pino added the Github action Pull requests that update GitHub Actions code label Mar 6, 2025
hudem1
hudem1 reviewed Mar 7, 2025
View reviewed changes
.github/workflows/starknet-rs-tests.yml
@@ -25,7 +25,7 @@ jobs:

- name: Run jsonrpc tests
run: |
cd starknet-providers && cargo test jsonrpc -- --skip jsonrpc_get_block_with_receipts
cd starknet-providers && cargo test jsonrpc
cd ../starknet-accounts && cargo test jsonrpc -- --skip can_declare_cairo0_contract_with_jsonrpc
Copy link
Contributor

@hudem1 hudem1 Mar 7, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

don't we wanna un-skip that test too ?

Copy link
Contributor

@hudem1 hudem1 Mar 7, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

Oh yeah okay we can't declare cairo 0 contract anymore. And the test is preceded by: #[ignore = "Cairo 0 contract declaration has been blocked"]

@hudem1
Copy link
Contributor

hudem1 commented Mar 7, 2025 •
edited
Loading

The starknet-rs test does not seem like a bug actually.

I checked it and from what i've seen, starknet-rs uses predeployed accounts to run tests. It seems like they use one specific account per test (at least that's what i saw in the test file single_owner_account.rs where the test fails). The test is failing simply because they give a gas * gas_price too high, equal to 90_000_000_000_000_000_000 FRI, meaning 90 STRK, (idk why they set it that high) and the account does not have enough funds.

I created a test account, funded it, ran the test with my account and it works with no problem.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@hudem1 hudem1 hudem1 left review comments

At least 2 approving reviews are required to merge this pull request.

Assignees
No one assigned
Labels
Github action Pull requests that update GitHub Actions code
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@wojciechos @hudem1 @rodrigo-pino