-
Notifications
You must be signed in to change notification settings - Fork 450
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
Display extra pages in goal info commands #2763
Display extra pages in goal info commands #2763
Conversation
Codecov Report
@@ Coverage Diff @@
## master #2763 +/- ##
==========================================
- Coverage 47.12% 47.09% -0.03%
==========================================
Files 349 349
Lines 56326 56348 +22
==========================================
- Hits 26541 26535 -6
- Misses 26814 26838 +24
- Partials 2971 2975 +4
Continue to review full report at Codecov.
|
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.
It seems good to me, assuming I've properly understood why there's so much churn around unrelated things (because of upgrade to v2, I'm assuming).
if [[ $RES != *"${PROGHASH}"* ]]; then | ||
date '+app-extra-pages-test FAIL the application info should succeed %Y%m%d_%H%M%S' | ||
date '+app-extra-pages-test FAIL the application approval program hash is incorrect %Y%m%d_%H%M%S' |
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.
Might be nice to migrate toward doing this at the top of e2e .sh programs
filename=$(basename "$0")
scriptname="${filename%.*}"
date "+${scriptname} start %Y%m%d_%H%M%S"
So you can use $scriptname here (and elsewhere). No big deal, just makes copying testing for modification easier.
@@ -31,7 +31,7 @@ import ( | |||
"github.com/algorand/go-algorand/config" | |||
"github.com/algorand/go-algorand/crypto" | |||
"github.com/algorand/go-algorand/crypto/passphrase" | |||
v1 "github.com/algorand/go-algorand/daemon/algod/api/spec/v1" | |||
generatedV2 "github.com/algorand/go-algorand/daemon/algod/api/server/v2/generated" |
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.
Can you summarize what we're doing with versioning that forces changes in this PR? We don't have to increment the version for changes like this, right? Maybe you're just migrating to the new version that we previously had to do?
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.
Actually I had to upgrade to using the v2 endpoint because v1 does not return extra pages values for accounts.
for _, createdAsset := range createdAssets { | ||
name := "<unnamed>" | ||
if createdAsset.Params.Name != nil { | ||
_, name = unicodePrintable(*createdAsset.Params.Name) |
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.
It may be unnecessary to pipe the string fields through unicodePrintable
after #2555, but I've left them in just in case.
Summary
With this PR, the commands
goal app info
,goal account info
, andgoal account list --info
now display the number of extra pages an application has.For example, the following were executed on a private network where apps 1, 2, 3, 4 have 1, 2, 3, 0 extra pages, respectively.
Test Plan
Modified existing e2e test and expect test. (I know expect tests aren't currently running, but it passed locally.)