-
Notifications
You must be signed in to change notification settings - Fork 373
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
chore(ci): run gno test with --print-runtime-metrics #2979
chore(ci): run gno test with --print-runtime-metrics #2979
Conversation
Signed-off-by: moul <[email protected]>
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #2979 +/- ##
=======================================
Coverage 63.39% 63.39%
=======================================
Files 565 565
Lines 79457 79457
=======================================
+ Hits 50369 50372 +3
+ Misses 25695 25693 -2
+ Partials 3393 3392 -1
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
Removing the draft status allows people to see how they can help. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Root Cause:
Without the flag, no Allocator is assigned to the test machine. By default, the MaxAlloc is unlimited.
However, with the --print-runtime-metrics flag, MaxAlloc is set to 500M.
Line 293 in f6bd2d3
maxAllocTx := int64(500 * 1000 * 1000) |
The test ./examples/gno.land/p/demo/diff/TestMyersDiff allocates 65.1G in the Allocator:
func TestMyersDiff(t *testing.T) { |
As a result, it panics and fails.
Options:
We have a two options:
- Increase MaxAlloc to 100G: maxAllocTx := int64(100000 * 1000 * 1000) with the --print-runtime-metrics flag
- Modify the test cases to fit within the current chain default configuration of 500M: maxAllocTx := int64(500 * 1000 * 1000)
Here are the results with the 100G MaxAlloc setting, which allows the test to complete:
- The last two test cases took 4.26s and 22.61s respectively on a local machine.
// There are a few other simple testing cases before this in TestMyersDiff/
=== RUN TestMyersDiff/Worst-case_scenario_(completely_different_strings)
--- PASS: TestMyersDiff/Worst-case_scenario_(completely_different_strings) (4.26s)
=== RUN TestMyersDiff/Very_long_strings
--- PASS: TestMyersDiff/Very_long_strings (22.61s)
--- runtime: cycle=218.7M imports=6 allocs=65.1G(65.12%)
I believe it makes sense to set much higher limits for unit tests since they won't be executed on-chain. These tests can serve as benchmarks and support other intensive evaluations. It might be good to have an "unlimited" or very high limits allocator by default for tests. We could include a warning when a specific test exceeds what a chain can support. This warning would be just that—a warning—because it is reasonable to have tests that are longer than actual on-chain operations. |
agreed, let's bump the limit to something like 1 << 63 so we keep track of the allocations without a limit. do you want to tackle it in this pr @moul? |
Signed-off-by: moul <[email protected]>
change applied, thx guys |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
note - approving this, #3026 is about go tests, this is about gno tests which for now i think make sense to be more verbose.
Enabling
--print-runtime-metrics
on the CI triggered a bug.One of our libraries passes without the flag but triggers a panic with the message
allocation limit exceeded
when the flag is present. I suspect this check occurs with thegno
CLI when the flag is enabled and also on-chain. Therefore, it may be a minor bug affecting local development, particularly for unit tests. Regardless, it deserves investigation.Diff too long (47Mb); embedding only the first and last 100 lines.