Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
caliper-ethereum : Add new tests for
connectorFactory
andethereum-connector
#1560base: main
Are you sure you want to change the base?
caliper-ethereum : Add new tests for
connectorFactory
andethereum-connector
#1560Changes from all commits
b581ee8
d40236f
9e010f6
e8e1151
4d3ff42
7912cf4
7ae2f49
52f90b3
3019394
66d930f
3209b25
63c45ff
7929e14
40a06d4
3d27fae
b2f4ae1
ffdb9f8
c8f2a2e
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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 test should be part of the connector factory. As stated before we don't want to have tests that directly test the constructor of the EthereumConnector as it isn't really a public constructor. Please move this test to the ConnectorFactory tests.
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 test should be part of the connector factory. As stated before we don't want to have tests that directly test the constructor of the EthereumConnector as it isn't really a public constructor. Please move this test to the ConnectorFactory tests.
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.
So this is actually a bug. The error message should not provide links like this as it means it needs to be updated with version changes. So we should fix the code and the test. Again this is an example where we should not assume that the code is correct and the test just passes because it is wrong as well. This is a good bug you have found though and simple to fix. We need to change the code to just say to refer to the ethereum configuration in the caliper documentation