-
Notifications
You must be signed in to change notification settings - Fork 3
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
add popTxs test #61
add popTxs test #61
Conversation
core/tx_test.go
Outdated
|
||
defer os.Remove(tmpfile.Name()) | ||
|
||
db, err := gorm.Open("sqlite", sqlite.Open("gorm.db")) |
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.
There is a error thrown here that wasn't caught due to which db is nil and hence the nil pointer dereference error.
invalid database source: &{gorm.db} is not a valid type
Above is what I get when I log it
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.
Thanks. It turns out I'm using sqlite from gorm V2 but our codebase uses gorm V1
@@ -0,0 +1,79 @@ | |||
package tests_test |
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.
Move to a tests/
package, because Golang complains a circular dependency between migration/
and core
.
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.
Sounds good, these are anyways exported functions.
return | ||
} | ||
|
||
sqliteDb, err := gorm.Open("sqlite3", tmpfile.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.
Sorry I eventually use SQLite because the setup is much easier than installing another docker instance in CI.
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.
This is very nice!
defer cleanup() | ||
|
||
var txType uint64 = 1 | ||
config.GlobalCfg.TxsPerBatch = 2 |
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.
Have to override this global variable to work.
for i, tx := range []core.Tx{tx2, tx3} { | ||
assert.Equal(t, tx.TxHash, txs[i].TxHash) | ||
} |
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.
I really want to do assert.Equal(t, []core.Tx{tx2, tx3}, txs)
here, because that allows us to have a whole view on the difference of the set of transactions.
Looping through each tx and comparing their hashes is more difficult to debug. I'm doing so because the Tx struct has a DBModel field, that makes the CreatedAt
value different between []core.Tx{tx2, tx3}
and txs.
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.
Interesting, I think we should have something like TxList that has a .equal()
and some other sorting, popping operations, similar to how geth does it.
What do you think?
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.
Have something like TxList means we are doing pending Txs in memory, not in disk/database? Let's move the discussion here #63
There are some more tests I would like to add for TestPopTx, such as checking if the txs are correctly updated to |
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.
Amazing work @ChihChengLiang! Lets merge!
run
go test ./core/ --run TestPopTx