fix(amino): panic when registering types with the same name
This adds some doc and tests to tm2/pkg/amino to address the following in amino_test.go:
fixed #2326
// XXX Test registering duplicate names or concrete types not in a package.
- chore(docs): document frequent functions in tm2/pkg/amino
- chore(amino): add some tests
To run tests
cd tm2/pkg/amino
go test -v --run=WithPanic\$
Tests have uncovered a potential bug however in TestDupNamesMustPanic.
Opening an issue now to document this, with a possible fix.
// The following does NOT panic, but it should.
// assert.Panics(t, func() {
// myPkg.WithTypes(
// tests.EmptyStruct{}, "A",
// tests.PrimitivesStruct{}, "B",
// tests.ShortArraysStruct{}, "A", // Same name
// )
// })
Contributors' checklist...
- [x] Added new tests, or not needed, or not feasible
- [x] Provided an example (e.g. screenshot) to aid review or the PR is self-explanatory
- [x] Updated the official documentation or not needed
- [x] No breaking changes were made, or a
BREAKING CHANGE: xxxmessage was included in the description - [ ] Added references to related issues and PRs
- [x] Provided any useful hints for running manual tests
- [ ] Added new benchmarks to generated graphs, if any. More info here.
Codecov Report
All modified and coverable lines are covered by tests :white_check_mark:
:loudspeaker: Thoughts on this report? Let us know!
Hello @grepsuzette . I do the following to clone your branch, merge master and run tests.
git clone https://github.com/grepsuzette/gno --branch amino_doc
cd gno
git remote add upstream https://github.com/gnolang/gno
git fetch upstream
git merge upstream/master
make test.components
It prints the test failure:
--- FAIL: TestStdlibs (0.01s)
--- FAIL: TestStdlibs/regexp/syntax (0.00s)
panic: StaticBlock.Define2(byte) cannot change .V [recovered]
panic: StaticBlock.Define2(byte) cannot change .V
goroutine 36 [running]:
testing.tRunner.func1.2({0x1049d1620, 0x1400010e090})
/Users/jefft0/.asdf/installs/golang/1.22.5/go/src/testing/testing.go:1631 +0x1c4
testing.tRunner.func1()
/Users/jefft0/.asdf/installs/golang/1.22.5/go/src/testing/testing.go:1634 +0x33c
panic({0x1049d1620?, 0x1400010e090?})
/Users/jefft0/.asdf/installs/golang/1.22.5/go/src/runtime/panic.go:770 +0x124
github.com/gnolang/gno/gnovm/pkg/gnolang.(*StaticBlock).Define2(0x1400019a330, 0x0, {0x104752bef, 0x4}, {0x104acb050, 0x104faf280}, {{0x104acb050, 0x104faf280}, {0x104ac4990, 0x1400010e070}, ...})
/Users/jefft0/temp/t2/gno/gnovm/pkg/gnolang/nodes.go:1848 +0x7f0
github.com/gnolang/gno/gnovm/pkg/gnolang.(*StaticBlock).Define(...)
/Users/jefft0/temp/t2/gno/gnovm/pkg/gnolang/nodes.go:1788
github.com/gnolang/gno/gnovm/pkg/gnolang.UverseNode.func1(...)
/Users/jefft0/temp/t2/gno/gnovm/pkg/gnolang/uverse.go:89
github.com/gnolang/gno/gnovm/pkg/gnolang.UverseNode()
/Users/jefft0/temp/t2/gno/gnovm/pkg/gnolang/uverse.go:100 +0x470
github.com/gnolang/gno/gnovm/pkg/gnolang.Uverse()
/Users/jefft0/temp/t2/gno/gnovm/pkg/gnolang/uverse.go:71 +0x28
github.com/gnolang/gno/gnovm/pkg/gnolang.InitStoreCaches({0x104ad8b88, 0x1400041a000})
/Users/jefft0/temp/t2/gno/gnovm/pkg/gnolang/store.go:866 +0x228
github.com/gnolang/gno/gnovm/pkg/gnolang.NewStore(0x0, {0x104acb950, 0x1400010e010}, {0x104acb9a8, 0x14000128120})
/Users/jefft0/temp/t2/gno/gnovm/pkg/gnolang/store.go:125 +0x128
command-line-arguments.TestStore({0x140004cc000, 0x5}, {0x140003ec780, 0xd}, {0x104ac1f08, 0x140004ca000}, {0x104ac1e68, 0x1400018e020}, {0x104ac1f28, 0x140004ca030}, ...)
/Users/jefft0/temp/t2/gno/gnovm/tests/imports.go:426 +0x228
command-line-arguments.runPackageTest(0x140003e0680, {0x14000400000, 0x18}, {0x140003ec780, 0xd})
/Users/jefft0/temp/t2/gno/gnovm/tests/package_test.go:74 +0x10c
command-line-arguments.TestStdlibs.func2(0x140003e0680)
/Users/jefft0/temp/t2/gno/gnovm/tests/package_test.go:58 +0x58
testing.tRunner(0x140003e0680, 0x14000406cf0)
/Users/jefft0/.asdf/installs/golang/1.22.5/go/src/testing/testing.go:1689 +0xec
created by testing.(*T).Run in goroutine 34
/Users/jefft0/.asdf/installs/golang/1.22.5/go/src/testing/testing.go:1742 +0x318
FAIL command-line-arguments 0.310s
FAIL
make[1]: *** [_test.gnolang.pkg1] Error 1
make: *** [test.components] Error 2
I don't know what went on there, @jefft0, but it seems to work now.
Anyway I fixed the bug involved with the text. Approving this PR, looking for another approval.
Removed the review/triage-pending label because this is approved by thehowl.
This PR is stale because it has been open 3 months with no activity. Remove stale label or comment or this will be closed in 3 months.
🛠 PR Checks Summary
All Automated Checks passed. ✅
Manual Checks (for Reviewers):
- [ ] IGNORE the bot requirements for this PR (force green CI check)
- [x] The pull request description provides enough details (checked by @thehowl)
Read More
🤖 This bot helps streamline PR reviews by verifying automated checks and providing guidance for contributors and reviewers.
✅ Automated Checks (for Contributors):
🟢 Maintainers must be able to edit this pull request (more info) 🟢 Pending initial approval by a review team member, or review from tech-staff
☑️ Contributor Actions:
- Fix any issues flagged by automated checks.
- Follow the Contributor Checklist to ensure your PR is ready for review.
- Add new tests, or document why they are unnecessary.
- Provide clear examples/screenshots, if necessary.
- Update documentation, if required.
- Ensure no breaking changes, or include
BREAKING CHANGEnotes. - Link related issues/PRs, where applicable.
☑️ Reviewer Actions:
- Complete manual checks for the PR, including the guidelines and additional checks if applicable.
📚 Resources:
Debug
Automated Checks
Maintainers must be able to edit this pull request (more info)
If
🟢 Condition met └── 🟢 And ├── 🟢 The base branch matches this pattern: ^master$ └── 🟢 The pull request was created from a fork (head branch repo: grepsuzette/gno)Then
🟢 Requirement satisfied └── 🟢 Maintainer can modify this pull requestPending initial approval by a review team member, or review from tech-staff
If
🟢 Condition met └── 🟢 And ├── 🟢 The base branch matches this pattern: ^master$ └── 🟢 Not (🔴 Pull request author is a member of the team: tech-staff)Then
🟢 Requirement satisfied └── 🟢 If ├── 🟢 Condition │ └── 🟢 Or │ ├── 🟢 At least 1 user(s) of the organization reviewed the pull request (with state "APPROVED") │ ├── 🟢 At least 1 user(s) of the team tech-staff reviewed pull request │ └── 🔴 This pull request is a draft └── 🟢 Then └── 🟢 Not (🔴 This label is applied to pull request: review/triage-pending)Manual Checks
**IGNORE** the bot requirements for this PR (force green CI check)
If
🟢 Condition met └── 🟢 On every pull requestCan be checked by
- Any user with comment edit permission
The pull request description provides enough details
If
🟢 Condition met └── 🟢 And ├── 🟢 Not (🔴 Pull request author is a member of the team: core-contributors) └── 🟢 Not (🔴 Pull request author is user: dependabot[bot])Can be checked by
- team core-contributors