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

db/memdb: failing tests and non-compiling code from "cannot use (*MemDB)(nil) (value of type *MemDB) as db.DBConnection value" #10319

Closed
3 of 4 tasks
odeke-em opened this issue Oct 7, 2021 · 6 comments · Fixed by #10325
Closed
3 of 4 tasks

db/memdb: failing tests and non-compiling code from "cannot use (*MemDB)(nil) (value of type *MemDB) as db.DBConnection value" #10319

odeke-em opened this issue Oct 7, 2021 · 6 comments · Fixed by #10325
Assignees
@roysc

Comments

@odeke-em
Copy link
Collaborator

odeke-em commented Oct 7, 2021

After this PR #9952 we now see this bizarre failure to compile

$ go test
# github.com/cosmos/cosmos-sdk/db/memdb [github.com/cosmos/cosmos-sdk/db/memdb.test]
./db.go:44:23: incompatible type: cannot use (*MemDB)(nil) (value of type *MemDB) as db.DBConnection value
./db_test.go:32:9: incompatible type: cannot use NewDB() (value of type *MemDB) as db.DBConnection value
FAIL	github.com/cosmos/cosmos-sdk/db/memdb [build failed]

which makes me wonder how this code hasn't been caught by CI for this long since August 31st 2021 and it is now October 6th 2021.

Kindly cc-ing @roysc

For Admin Use

  • Not duplicate issue
  • Appropriate labels applied
  • Appropriate contributors tagged
  • Contributor assigned/self-assigned
@roysc
Copy link
Contributor

roysc commented Oct 7, 2021 •
edited
Loading

Trying to reproduce - this is a local failure from running go test within db/memdb right? Strange that it doesn't list any missing interface method as it usually does.

The only failures I get when running the whole SDK test suite locally are from ledger_test.go - support for ledger devices is not available in this executable which is surely unrelated. I'm using Go 1.17.1 on Linux

@roysc
Copy link
Contributor

roysc commented Oct 7, 2021

Go 1.16.3 is also passing

@odeke-em
Copy link
Collaborator Author

odeke-em commented Oct 7, 2021 •
edited
Loading

Trying to reproduce - this is a local failure from running go test within db/memdb right? Strange that it doesn't list any missing interface method as it usually does.

The only failures I get when running the whole SDK test suite locally are from ledger_test.go - support for ledger devices is not available in this executable which is surely unrelated. I'm using Go 1.17.1 on Linux

Hehe I think that you might be using the latest Go (which will be Go1.18) and if so then you've encountered this Go compiler bug golang/go#48833 or golang/go#48471 which I am going to fix soon. But otherwise I can reproduce this db/memdb bug locally and even on Bencher running on Go1.17 per https://dashboard.github.orijtech.com/benchmark/fd23fc6ba89d474a8f7e821d3d343761

@roysc
Copy link
Contributor

roysc commented Oct 7, 2021

Ah, I see. Sorry I was testing at the merge commit for the memdb backend, not master. Yes, the Revert method was accidentally merged in this PR before the Memdb and Badgerdb implementations, which are coming in #10308. These should have been committed together.

@robert-zaremba
Copy link
Collaborator

@roysc , this issue will be closed with #10308, right?

@robert-zaremba robert-zaremba mentioned this issue Oct 7, 2021
Merged
19 tasks
@robert-zaremba
Copy link
Collaborator

@odeke-em - indeed, there must be something wrong with the CI. I'm investigating it in this PR: #10325

@odeke-em odeke-em mentioned this issue Oct 16, 2021
Closed
@mergify mergify bot closed this as completed in #10325 Oct 28, 2021
mergify bot pushed a commit that referenced this issue Oct 28, 2021
@robert-zaremba
fix: github test workflow (#10325)
3c3704f 
## Description
Closes:  #10319

@odeke-em  reported that `db/memdb` doesn't build: #10319
The problem is not detected by the Github CI. Here we try to fix this.

---

### Author Checklist

*All items are required. Please add a note to the item if the item is not applicable and
please add links to any relevant follow up issues.*

I have...

- [x] included the correct [type prefix](https://github.com/commitizen/conventional-commit-types/blob/v3.0.0/index.json) in the PR title
- [x] added `!` to the type prefix if API or client breaking change
- [x] targeted the correct branch (see [PR Targeting](https://github.com/cosmos/cosmos-sdk/blob/master/CONTRIBUTING.md#pr-targeting))
- [x] provided a link to the relevant issue or specification
- [x] followed the guidelines for [building modules](https://github.com/cosmos/cosmos-sdk/blob/master/docs/building-modules)
- [x] included the necessary unit and integration [tests](https://github.com/cosmos/cosmos-sdk/blob/master/CONTRIBUTING.md#testing)
- [x] added a changelog entry to `CHANGELOG.md`
- [x] included comments for [documenting Go code](https://blog.golang.org/godoc)
- [x] updated the relevant documentation or specification
- [ ] reviewed "Files changed" and left comments if necessary
- [ ] confirmed all CI checks have passed

### Reviewers Checklist

*All items are required. Please add a note if the item is not applicable and please add
your handle next to the items reviewed if you only reviewed selected items.*

I have...

- [ ] confirmed the correct [type prefix](https://github.com/commitizen/conventional-commit-types/blob/v3.0.0/index.json) in the PR title
- [ ] confirmed `!` in the type prefix if API or client breaking change
- [ ] confirmed all author checklist items have been addressed 
- [ ] reviewed state machine logic
- [ ] reviewed API design and naming
- [ ] reviewed documentation is accurate
- [ ] reviewed tests and test coverage
- [ ] manually tested (if applicable)
creachadair pushed a commit that referenced this issue Oct 30, 2021
@robert-zaremba @creachadair
fix: github test workflow (#10325)
f2e38f6 
## Description
Closes:  #10319

@odeke-em  reported that `db/memdb` doesn't build: #10319
The problem is not detected by the Github CI. Here we try to fix this.

---

### Author Checklist

*All items are required. Please add a note to the item if the item is not applicable and
please add links to any relevant follow up issues.*

I have...

- [x] included the correct [type prefix](https://github.com/commitizen/conventional-commit-types/blob/v3.0.0/index.json) in the PR title
- [x] added `!` to the type prefix if API or client breaking change
- [x] targeted the correct branch (see [PR Targeting](https://github.com/cosmos/cosmos-sdk/blob/master/CONTRIBUTING.md#pr-targeting))
- [x] provided a link to the relevant issue or specification
- [x] followed the guidelines for [building modules](https://github.com/cosmos/cosmos-sdk/blob/master/docs/building-modules)
- [x] included the necessary unit and integration [tests](https://github.com/cosmos/cosmos-sdk/blob/master/CONTRIBUTING.md#testing)
- [x] added a changelog entry to `CHANGELOG.md`
- [x] included comments for [documenting Go code](https://blog.golang.org/godoc)
- [x] updated the relevant documentation or specification
- [ ] reviewed "Files changed" and left comments if necessary
- [ ] confirmed all CI checks have passed

### Reviewers Checklist

*All items are required. Please add a note if the item is not applicable and please add
your handle next to the items reviewed if you only reviewed selected items.*

I have...

- [ ] confirmed the correct [type prefix](https://github.com/commitizen/conventional-commit-types/blob/v3.0.0/index.json) in the PR title
- [ ] confirmed `!` in the type prefix if API or client breaking change
- [ ] confirmed all author checklist items have been addressed 
- [ ] reviewed state machine logic
- [ ] reviewed API design and naming
- [ ] reviewed documentation is accurate
- [ ] reviewed tests and test coverage
- [ ] manually tested (if applicable)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@roysc roysc

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

fix: github test workflow cosmos/cosmos-sdk
fix: .github/workflows retrofit to use Go modules
3 participants
@robert-zaremba @odeke-em @roysc