-
Notifications
You must be signed in to change notification settings - Fork 2.3k
Internal improvement: Move db integration test to truffle package #5640
Conversation
24e8fae
to
f841fbf
Compare
b101b36
to
d0609e9
Compare
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.
Yay, an improved db test! Looks good to me!
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 ran yarn test
on packages/truffle and observed that the db test is executed successfully
truffle db
bigint: Failed to load bindings, pure JS will be used (try npm run rebuild?)
bigint: Failed to load bindings, pure JS will be used (try npm run rebuild?)
✔ creates a project
✔ retrieves contract instances by project (82ms)
✔ retrieves saved compilations
✔ creates contract records
✔ loads contract sources
✔ loads bytecodes
However, there are dependency warning message:
$ yarn test
yarn run v1.22.11
warning package.json: "optionalDependencies" has dependency "@truffle/db" with range "^2.0.3" that collides with a dependency in "devDependencies" of the same name with version "^2.0.2"
$ ./scripts/test.sh
warning package.json: "optionalDependencies" has dependency "@truffle/db" with range "^2.0.3" that collides with a dependency in "devDependencies" of the same name with version "^2.0.2"
$ /Users/dongminghwang/work/trufflesuite/truffle/node_modules/.bin/semver -r '>=12' v16.15.0
16.15.0
warning package.json: "optionalDependencies" has dependency "@truffle/db" with range "^2.0.3" that collides with a dependency in "devDependencies" of the same name with version "^2.0.2"
$ yarn build:cli
warning package.json: "optionalDependencies" has dependency "@truffle/db" with range "^2.0.3" that collides with a dependency in "devDependencies" of the same name with version "^2.0.2"
Oh, looks like versions in |
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'm going to go ahead and hit request changes to make sure this doesn't get merged without the versions being fixed.
…nning CI against geth
d0609e9
to
041d535
Compare
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.
Re-test shows that the dependency issue is fixed. Thanks.
I like the idea of moving db integration test to truffle package.
The changes are good.
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.
Tests pass, versions synced, looks good. I like 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.
🎉
In order to remove a cyclic dependency in db (@truffle/db --> @truffle/migrate --> @truffle/db-loader --> @truffle/db) this PR moves and rewrites a db integration test and finds a new home for it in truffle.
This should retire #5600