Add ai and dev flag for codecove coverage report#107
Conversation
…sts name for testing
|
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the WalkthroughThis PR expands CI/CD test coverage by introducing three parallel coverage pipelines per package (all tests, AI-only, dev-only) with distinct Codecov flags. Two new test functions are added to validate scheduled transfer execution and safe coin downcast with same decimals. The .gitignore is updated to exclude lcov coverage and trace files. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes
Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
Comment |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #107 +/- ##
=======================================
Coverage 96.14% 96.14%
=======================================
Files 13 13
Lines 1193 1193
Branches 339 339
=======================================
Hits 1147 1147
Misses 29 29
Partials 17 17
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
math/core/tests/coin_utils_tests.move (1)
20-25: AI duplicate downcast test mirrors baseline behavior
ai_TMP_test_downcast_same_decimalsis equivalent totest_downcast_same_decimalsand correctly exercisessafe_downcast_balancefor same‑decimals (9→9) with the expectedu64result. This is appropriate as a temporary AI‑tagged duplicate for coverage.Once the AI/dev‑flagged coverage setup stabilizes, consider removing this temporary duplicate or refactoring to avoid drift between the two tests, e.g., via a shared helper.
Based on learnings, the
ai_prefix usage here is consistent with the repo’s convention for AI-generated tests.contracts/access/tests/delayed_tests.move (1)
24-45: AI variant of schedule/execute transfer test is consistent
ai_TMP_schedule_and_execute_transfermirrorsschedule_and_execute_transfer: same ownership/recipient setup, scheduling, clock advancement, execution, and event assertions, plus properclock::destroy_for_testingcleanup. It cleanly seeds the AI-only test flow without changing behavior.Later, once the coverage experiment is done, you might want to drop this temporary duplicate or factor the common logic into a helper to avoid keeping two copies in sync.
Based on learnings, the
ai_prefix is aligned with how this repo tracks AI-generated tests.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
.github/workflows/test.yml(3 hunks).gitignore(1 hunks)contracts/access/tests/delayed_tests.move(1 hunks)math/core/tests/coin_utils_tests.move(1 hunks)
🧰 Additional context used
🧠 Learnings (3)
📓 Common learnings
Learnt from: ericglau
Repo: OpenZeppelin/contracts-sui PR: 49
File: math/core/tests/u32_tests.move:455-486
Timestamp: 2025-11-18T18:31:53.671Z
Learning: In the OpenZeppelin contracts-sui repository, test function names prefixed with "ai_" are intentionally used to track tests generated by AI tools. This naming convention is acceptable and should not be flagged as inconsistent with project standards.
📚 Learning: 2025-11-04T14:38:12.309Z
Learnt from: ericnordelo
Repo: OpenZeppelin/contracts-sui PR: 40
File: math/core/sources/u8.move:50-57
Timestamp: 2025-11-04T14:38:12.309Z
Learning: In math/core/sources/{u8,u16,u32,u64,u128}.move files, the mul_shr implementations correctly ignore the overflow flag from macros::mul_shr! and rely on try_as_*() to detect when the result doesn't fit in the target type. Only u256.move needs to explicitly check the overflow flag because u256 operations can exceed u256 capacity.
<!-- [/add_learning]
Applied to files:
math/core/tests/coin_utils_tests.move
📚 Learning: 2025-11-18T18:31:53.671Z
Learnt from: ericglau
Repo: OpenZeppelin/contracts-sui PR: 49
File: math/core/tests/u32_tests.move:455-486
Timestamp: 2025-11-18T18:31:53.671Z
Learning: In the OpenZeppelin contracts-sui repository, test function names prefixed with "ai_" are intentionally used to track tests generated by AI tools. This naming convention is acceptable and should not be flagged as inconsistent with project standards.
Applied to files:
contracts/access/tests/delayed_tests.move
🪛 actionlint (1.7.9)
.github/workflows/test.yml
130-130: input "file" is not defined in action "codecov/codecov-action@v5". available inputs are "base_sha", "binary", "codecov_yml_path", "commit_parent", "directory", "disable_file_fixes", "disable_safe_directory", "disable_search", "disable_telem", "dry_run", "env_vars", "exclude", "fail_ci_if_error", "files", "flags", "force", "gcov_args", "gcov_executable", "gcov_ignore", "gcov_include", "git_service", "handle_no_reports_found", "job_code", "name", "network_filter", "network_prefix", "os", "override_branch", "override_build", "override_build_url", "override_commit", "override_pr", "plugins", "recurse_submodules", "report_code", "report_type", "root_dir", "run_command", "skip_validation", "slug", "swift_project", "token", "url", "use_legacy_upload_endpoint", "use_oidc", "use_pypi", "verbose", "version", "working-directory"
(action)
147-147: input "file" is not defined in action "codecov/codecov-action@v5". available inputs are "base_sha", "binary", "codecov_yml_path", "commit_parent", "directory", "disable_file_fixes", "disable_safe_directory", "disable_search", "disable_telem", "dry_run", "env_vars", "exclude", "fail_ci_if_error", "files", "flags", "force", "gcov_args", "gcov_executable", "gcov_ignore", "gcov_include", "git_service", "handle_no_reports_found", "job_code", "name", "network_filter", "network_prefix", "os", "override_branch", "override_build", "override_build_url", "override_commit", "override_pr", "plugins", "recurse_submodules", "report_code", "report_type", "root_dir", "run_command", "skip_validation", "slug", "swift_project", "token", "url", "use_legacy_upload_endpoint", "use_oidc", "use_pypi", "verbose", "version", "working-directory"
(action)
164-164: input "file" is not defined in action "codecov/codecov-action@v5". available inputs are "base_sha", "binary", "codecov_yml_path", "commit_parent", "directory", "disable_file_fixes", "disable_safe_directory", "disable_search", "disable_telem", "dry_run", "env_vars", "exclude", "fail_ci_if_error", "files", "flags", "force", "gcov_args", "gcov_executable", "gcov_ignore", "gcov_include", "git_service", "handle_no_reports_found", "job_code", "name", "network_filter", "network_prefix", "os", "override_branch", "override_build", "override_build_url", "override_commit", "override_pr", "plugins", "recurse_submodules", "report_code", "report_type", "root_dir", "run_command", "skip_validation", "slug", "swift_project", "token", "url", "use_legacy_upload_endpoint", "use_oidc", "use_pypi", "verbose", "version", "working-directory"
(action)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Test math/core
- GitHub Check: Test contracts/access
🔇 Additional comments (2)
.gitignore (1)
19-22: Coverage artifacts and traces correctly ignoredThe
**/*lcov*and**/tracespatterns line up with the new per‑package lcov reports and trace directories and will keep generated coverage data out of version control. No issues from a tooling perspective..github/workflows/test.yml (1)
153-155: Confirm dev-only test filter behaves as intended with Sui CLIThe dev-only test run uses:
run: sui move test --trace --coverage '::([^a][^:]*|a[^i][^:]*|ai[^_][^:]*)$'This regex is crafted to exclude functions whose names start with
ai_after the::separator (so AI-tagged tests only run in the AI flow). That logic looks correct on paper, but it relies on howsui move testinterprets the filter string (regex vs plain substring, what it matches against, etc.).I recommend quickly validating this locally by:
- Running
sui move test '::ai_'to confirm onlyai_tests run.- Running
sui move test '::([^a][^:]*|a[^i][^:]*|ai[^_][^:]*)$'and checking that noai_...tests are reported.
| on: | ||
| push: | ||
| branches: [main, v*] | ||
| branches: [main, v*, "codecove-ai-tests-flag"] | ||
| pull_request: | ||
| branches: [main, v*] |
There was a problem hiding this comment.
Branch filter typo will skip pushes on the intended branch
The push trigger is configured with "codecove-ai-tests-flag", but the PR source branch is codecov-ai-tests-flag (missing the extra “e”). That means pushes to the current PR branch will not trigger this workflow via push.
Recommend updating the branch name here to exactly match the actual branch you intend to use (likely codecov-ai-tests-flag).
🤖 Prompt for AI Agents
.github/workflows/test.yml around lines 3 to 7: the push trigger lists
"codecove-ai-tests-flag" which is misspelled and will not match your actual
branch name; update that entry to the correct branch name
"codecov-ai-tests-flag" (or whatever exact branch you intend) so pushes to the
PR source branch will trigger the workflow.
| - name: Upload coverage to Codecov (all) | ||
| uses: codecov/codecov-action@v5 | ||
| with: | ||
| file: ./${{ matrix.package }}/lcov-all.info | ||
| flags: all,${{ matrix.package }} | ||
| name: ${{ matrix.package }}-all | ||
| fail_ci_if_error: true | ||
| token: ${{ secrets.CODECOV_TOKEN }} | ||
|
|
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
codecov/codecov-action@v5 official documentation inputs
💡 Result:
The official inputs for codecov/codecov-action@v5 are defined in the repository's action.yml (see the "Arguments"/inputs section of the README). Do you want me to fetch and list every input and its description from action.yml now? (github.com)
Citations:
🏁 Script executed:
#!/bin/bash
# Fetch action.yml from codecov/codecov-action v5 tag
curl -s https://raw.githubusercontent.com/codecov/codecov-action/v5/action.yml | head -100Repository: OpenZeppelin/contracts-sui
Length of output: 4328
🏁 Script executed:
#!/bin/bash
# Check if `file` parameter exists in earlier versions (v4)
curl -s https://raw.githubusercontent.com/codecov/codecov-action/v4/action.yml 2>/dev/null | grep -A 2 "^ file:" || echo "file parameter not found in v4"Repository: OpenZeppelin/contracts-sui
Length of output: 148
Change file to files for codecov/codecov-action@v5
The action is configured with file, but codecov/codecov-action@v5 requires the input files (plural). This is a breaking change from v4. Using the incorrect parameter name will cause runtime failures.
Apply the fix to all three Codecov upload steps:
- name: Upload coverage to Codecov (all)
uses: codecov/codecov-action@v5
with:
- file: ./${{ matrix.package }}/lcov-all.info
+ files: ./${{ matrix.package }}/lcov-all.info
flags: all,${{ matrix.package }}
name: ${{ matrix.package }}-all
fail_ci_if_error: true
token: ${{ secrets.CODECOV_TOKEN }}
- name: Upload coverage to Codecov (ai)
uses: codecov/codecov-action@v5
with:
- file: ./${{ matrix.package }}/lcov-ai.info
+ files: ./${{ matrix.package }}/lcov-ai.info
flags: ai,${{ matrix.package }}
name: ${{ matrix.package }}-ai
fail_ci_if_error: true
token: ${{ secrets.CODECOV_TOKEN }}
- name: Upload coverage to Codecov (dev)
uses: codecov/codecov-action@v5
with:
- file: ./${{ matrix.package }}/lcov-dev.info
+ files: ./${{ matrix.package }}/lcov-dev.info
flags: dev,${{ matrix.package }}
name: ${{ matrix.package }}-dev
fail_ci_if_error: true
token: ${{ secrets.CODECOV_TOKEN }}Committable suggestion skipped: line range outside the PR's diff.
🧰 Tools
🪛 actionlint (1.7.9)
130-130: input "file" is not defined in action "codecov/codecov-action@v5". available inputs are "base_sha", "binary", "codecov_yml_path", "commit_parent", "directory", "disable_file_fixes", "disable_safe_directory", "disable_search", "disable_telem", "dry_run", "env_vars", "exclude", "fail_ci_if_error", "files", "flags", "force", "gcov_args", "gcov_executable", "gcov_ignore", "gcov_include", "git_service", "handle_no_reports_found", "job_code", "name", "network_filter", "network_prefix", "os", "override_branch", "override_build", "override_build_url", "override_commit", "override_pr", "plugins", "recurse_submodules", "report_code", "report_type", "root_dir", "run_command", "skip_validation", "slug", "swift_project", "token", "url", "use_legacy_upload_endpoint", "use_oidc", "use_pypi", "verbose", "version", "working-directory"
(action)
🤖 Prompt for AI Agents
In .github/workflows/test.yml around lines 127 to 135, the Codecov upload step
uses the deprecated/incorrect input name "file" instead of the required "files"
for codecov/codecov-action@v5; update the step(s) to replace the "file:" key
with "files:" (preserving the same value like ./${{ matrix.package
}}/lcov-all.info) and apply the same change to the other two Codecov upload
steps in the workflow so all three use "files" instead of "file".
…acts-sui into codecov-ai-tests-flag
| needs: [setup, lint] | ||
| runs-on: ubuntu-latest | ||
| timeout-minutes: 15 | ||
| timeout-minutes: 25 |
There was a problem hiding this comment.
Tests can take longer to run
| uses: codecov/codecov-action@v5 | ||
| with: | ||
| file: ./${{ matrix.package }}/lcov.info | ||
| files: ./${{ matrix.package }}/lcov-all.info |
There was a problem hiding this comment.
Why changed to file -> files?
There was a problem hiding this comment.
The API seem to mention files https://github.com/codecov/codecov-action
ericnordelo
left a comment
ericnordelo
left a comment
There was a problem hiding this comment.
Is there any strong benefit for adding this? Maybe this is helpful enough, I'm not sure at this point, but running the test suite with coverage generation three times seems costful. In Sui this is doable yet, but, for example, in our current cairo repo this is unfeasible since the tests suite is not the fastest and this workflow would not pass.
.gitignore
Outdated
|
|
||
| **/*lcov* | ||
|
|
||
| **/traces No newline at end of file |
|
@ericnordelo That is a good point, similarly to Defender OSS where we had to increase the container ressources for them to pass. I will surface this consideration. I believe all the repos will align to the goal of having coverage generation splitting ai generated tests and dev tests, we can probably find some optimization on the way. |
|
Sure, but I'm still curious about the benefit of it, since it adds so much overhead. |
3 participants
Add ai and dev flag for codecove coverage report
Temporary duplicate tests for testing
Summary by CodeRabbit
Tests
Chores
✏️ Tip: You can customize this high-level summary in your review settings.