-
Notifications
You must be signed in to change notification settings - Fork 1
TrackerRegistry #40
TrackerRegistry #40
Conversation
src/utils/TrackerRegistry.js
Outdated
contractAddress, trackerRegistryConfig, jsonRpcProvider, servers, algorithm = 'sha256', hashRingOptions | ||
}) => { | ||
const trackerRegistry = new TrackerRegistry({ | ||
contractAddress, trackerRegistryConfig, jsonRpcProvider, servers, algorithm, hashRingOptions |
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.
Is the trackerRegistryConfig
really necessary as a param? If it lives in this repo it could just use it by default?
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.
If the only difference between dev
, staging
and prod
is contractAddress
- yes, sure. But who can clarify this?
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.
Yea, I don't know either... just thinking about the usage of the helper that it's an extra step if it can already assume that it's the contract it will be using. But ok for me to go with this as well.
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.
late review, lgtm, left a PSA about jest async + done.
]) | ||
}) | ||
|
||
test('throw exception if address is wrong (ENS)', async (done) => { |
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.
FYI async (done)
is going away when jest-circus
lands:
jestjs/jest#9129
Scheduled for Jest 27:
jestjs/jest#6295
I opened a related issue here:
jestjs/jest#10529
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.
To get rid of the done
the equivalent would probably be this:
await expect(await getTrackerRegistry({
contractAddress: 'address', jsonRpcProvider
})).rejects.toThrow('Error: network does not support ENS')
TrackerRegistry
CI
integration tests (usingstreamr-docker-dev
)npm scripts