caliper-ethereum : Add new tests for connectorFactory and ethereum-connector
#1560
Conversation
Signed-off-by: Abhinav Pandey <[email protected]>
Signed-off-by: Abhinav Pandeey <[email protected]> Signed-off-by: Abhinav Pandey <[email protected]>
Signed-off-by: Abhinav Pandey <[email protected]>
|
@aklenik Can you take a look ? |
Signed-off-by: Abhinav Pandey <[email protected]>
Signed-off-by: Abhinav Pandey <[email protected]>
Signed-off-by: Abhinav Pandey <[email protected]>
Signed-off-by: Abhinav Pandey <[email protected]>
Signed-off-by: Abhinav Pandey <[email protected]>
|
Hey, Sorry for the inactivity. I've been busy from the past few days due to travel. I'll wrap this up soon. |
Signed-off-by: Abhinav Pandey <[email protected]>
|
I've been encountering an error : |
Temporarily removed tests for installSmartContracts for debugging purposes Signed-off-by: Abhinav Pandey <[email protected]>
Any Updates on this? Suites Prepared. Can you review the suites now? |
Signed-off-by: Abhinav Pandey <[email protected]>
Signed-off-by: Abhinav Pandey <[email protected]>
Signed-off-by: Abhinav Pandey <[email protected]>
| './sample-configs/invalidUrlConfig.json' | ||
| ); | ||
| ConfigUtil.set(ConfigUtil.keys.NetworkConfig, invalidConfig); | ||
| expect(() => new EthereumConnector(invalidConfig)).to.throw( |
There was a problem hiding this comment.
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.
| }); | ||
|
|
||
| describe('When constructed with absent url path', function () { | ||
| it('should throw an error', function () { |
There was a problem hiding this comment.
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.
| ConfigUtil.set(ConfigUtil.keys.NetworkConfig, invalidConfig); | ||
| expect(() => new EthereumConnector(invalidConfig)).to.throw( | ||
| 'No URL given to access the Ethereum SUT. Please check your network configuration. ' + | ||
| 'Please see https://hyperledger.github.io/caliper/v0.3/ethereum-config/ for more info.' |
There was a problem hiding this comment.
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
Added test cases for
caliper-ethereumpackageReady for Review
Checklist
Issue/User story
Steps to Reproduce
cd packages/caliper-ethereum && npm i && npm run devExisting issues
Design of the fix
Validation of the fix
Automated Tests
What documentation has been provided for this pull request