Skip to content
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

Fix #536 Update error messages for 'caliper-core' package #1362

Closed
wants to merge 5 commits into from

Conversation

0xt3j4s
Copy link
Contributor

@0xt3j4s 0xt3j4s commented May 31, 2022

  • Partially fixes # 536. (Intentionally not linked this PR to the issue because as of now it only partially fixes it)
  • The error messages are clear and consistent.
  • All the error messages i.e. at debug level, error level, and info level are checked and updated.

Signed-off-by: Tezas-6174 <jamdade.2@iitj.ac.in>
Signed-off-by: Tezas-6174 <jamdade.2@iitj.ac.in>
Signed-off-by: Tezas-6174 <jamdade.2@iitj.ac.in>
Signed-off-by: Tejas <jamdade.2@iitj.ac.in>
@davidkel
Copy link
Contributor

The build shows 5 test failures

 1) chart builder implementation #retrieveIncludedMetrics should log an error and return an empty array if not provided any metrics:
     AssertError: expected error to be called with arguments 
Required "metrics" not provided for bar chart generation for monitor: CallingMonitor Required "metrics" not provided for bar chart generation for monitor CallingMonitor 
      at Object.fail (/home/vsts/work/1/s/node_modules/sinon/lib/sinon/assert.js:106:21)
      at failAssertion (/home/vsts/work/1/s/node_modules/sinon/lib/sinon/assert.js:65:16)
      at Object.assert.<computed> [as calledWith] (/home/vsts/work/1/s/node_modules/sinon/lib/sinon/assert.js:91:13)
      at Context.<anonymous> (test/manager/charts/chart-builder.js:85:26)
      at processImmediate (internal/timers.js:464:21)

  2) chart builder implementation #retrieveIncludedMetrics should log an error and return an empty array if the "all" option is listed with other metrics:
     AssertError: expected error to be called with arguments 
Cannot list "all" option with other metrics for bar chart generation for monitor: CallingMonitor Cannot list "all" option with other metrics for bar chart generation for monitor CallingMonitor 
      at Object.fail (/home/vsts/work/1/s/node_modules/sinon/lib/sinon/assert.js:106:21)
      at failAssertion (/home/vsts/work/1/s/node_modules/sinon/lib/sinon/assert.js:65:16)
      at Object.assert.<computed> [as calledWith] (/home/vsts/work/1/s/node_modules/sinon/lib/sinon/assert.js:91:13)
      at Context.<anonymous> (test/manager/charts/chart-builder.js:96:26)
      at processImmediate (internal/timers.js:464:21)

  3) chart builder implementation ChartBuilder.retrieveChartStats should call log error and return empty array if unknown chart type:
     AssertError: expected error to be called with arguments 
Unknown chart type "unknown" requested Unknown chart type named "unknown" requested 
      at Object.fail (/home/vsts/work/1/s/node_modules/sinon/lib/sinon/assert.js:106:21)
      at failAssertion (/home/vsts/work/1/s/node_modules/sinon/lib/sinon/assert.js:65:16)
      at Object.assert.<computed> [as calledWith] (/home/vsts/work/1/s/node_modules/sinon/lib/sinon/assert.js:91:13)
      at Context.<anonymous> (test/manager/charts/chart-builder.js:189:26)
      at processImmediate (internal/timers.js:464:21)

  4) Formatted String Value Provider Constructor should throw an error for undefined format:
     AssertionError: expected [Function: wrapper] to throw error including 'Invalid format value: undefined' but got 'The format of options is invalid. Current format: undefined'
      at Context.<anonymous> (test/worker/workload/declarative/value-providers/formatted-string-value-provider.test.js:39:37)
      at processImmediate (internal/timers.js:464:21)

  5) Formatted String Value Provider Constructor should throw an error for non-string format:
     AssertionError: expected [Function: wrapper] to throw error including 'Invalid format value: 1' but got 'The format of options is invalid. Current format: 1'
      at Context.<anonymous> (test/worker/workload/declarative/value-providers/formatted-string-value-provider.test.js:49:37)
      at processImmediate (internal/timers.js:464:21)

from the build output (but you will also get the failures if you run the tests locally, ie npm test in the caliper directory (to run the unit tests in all the packages) or npm test in the calipre-core directory (to run the unit tests for caliper-core only)

The above tells you the failing line for each of the tests

1)      at Context.<anonymous> (test/manager/charts/chart-builder.js:85:26)
2)      at Context.<anonymous> (test/manager/charts/chart-builder.js:96:26)
3)      at Context.<anonymous> (test/manager/charts/chart-builder.js:189:26)
4)      at Context.<anonymous> (test/worker/workload/declarative/value-providers/formatted-string-value-provider.test.js:39:37)
5)      at Context.<anonymous> (test/worker/workload/declarative/value-providers/formatted-string-value-provider.test.js:49:37)

These tests are failing because they are looking for the exact error message which has now been changed by the PR so the tests need to be updated

It's highly recommended that you ensure the builds pass (both unit and integration) on your machine first before submitting a PR, but you need to ensure your PR passes the build otherwise a maintainer is likely to ignore it until it is fixed.

@0xt3j4s
Copy link
Contributor Author

0xt3j4s commented Jun 21, 2022

Yes but I am unable to run the tests locally, as it gives an error as follows:

$npm test

> caliper@0.5.0 pretest
> npm run licchk


> caliper@0.5.0 licchk
> license-check-and-add

Running using exclude exact_paths list
Running using exclude file type list
Trailing whitespace will be ignored in checking
No default format specified using {"prepend":"/*","append":"*/"} as backup
✔ All files have licenses.

> caliper@0.5.0 test
> lerna run test

lerna notice cli v3.22.1
lerna info Executing command in 10 packages: "npm run test"
lerna ERR! npm run test exited 217 in '@hyperledger/generator-caliper'
lerna ERR! npm run test stdout:

> @hyperledger/generator-caliper@0.5.0 pretest
> npm run licchk


> @hyperledger/generator-caliper@0.5.0 licchk
> license-check-and-add

Running using exclude exact_paths list
Running using exclude file type list
Trailing whitespace will be ignored in checking
✔ All files have licenses.

> @hyperledger/generator-caliper@0.5.0 test
> npm run lint && npm run nyc


> @hyperledger/generator-caliper@0.5.0 lint
> npx eslint .


lerna ERR! npm run test stderr:
No default format specified using {"prepend":"/*","append":"*/"} as backup
npm WARN exec The following package was not found and will be installed: eslint
npm ERR! code ENOTEMPTY
npm ERR! syscall rename
npm ERR! path /home/tejas/.npm/_npx/515228b7c8d004a2/node_modules/eslint
npm ERR! dest /home/tejas/.npm/_npx/515228b7c8d004a2/node_modules/.eslint-W5wbRWoE
npm ERR! errno -39
npm ERR! ENOTEMPTY: directory not empty, rename '/home/tejas/.npm/_npx/515228b7c8d004a2/node_modules/eslint' -> '/home/tejas/.npm/_npx/515228b7c8d004a2/node_modules/.eslint-W5wbRWoE'

npm ERR! A complete log of this run can be found in:
npm ERR!     /home/tejas/.npm/_logs/2022-06-22T01_36_46_494Z-debug.log
npm ERR! code 217
npm ERR! path /home/tejas/Documents/Projects/Hyperledger_caliper/caliper/packages/generator-caliper
npm ERR! command failed
npm ERR! command sh -c npx eslint .

npm ERR! A complete log of this run can be found in:
npm ERR!     /home/tejas/.npm/_logs/2022-06-22T01_36_46_508Z-debug.log
npm ERR! code 217
npm ERR! path /home/tejas/Documents/Projects/Hyperledger_caliper/caliper/packages/generator-caliper
npm ERR! command failed
npm ERR! command sh -c npm run lint && npm run nyc

npm ERR! A complete log of this run can be found in:
npm ERR!     /home/tejas/.npm/_logs/2022-06-22T01_36_46_521Z-debug.log

lerna ERR! npm run test exited 217 in '@hyperledger/generator-caliper'
lerna WARN complete Waiting for 3 child processes to exit. CTRL-C to exit immediately.
npm ERR! code 217
npm ERR! path /home/tejas/Documents/Projects/Hyperledger_caliper/caliper
npm ERR! command failed
npm ERR! command sh -c lerna run test

npm ERR! A complete log of this run can be found in:
npm ERR!     /home/tejas/.npm/_logs/2022-06-22T01_36_46_544Z-debug.log

I had talked to @aklenik about this, and then also updated the node version; still, it is not working as expected.
That's why I am manually checking the changes I have made and then they are tested here in this repository.

Signed-off-by: Tezas-6174 <jamdade.2@iitj.ac.in>
@aklenik
Copy link
Contributor

aklenik commented Jun 28, 2022

@Tezas-6174 The build still shows an error:

1) Formatted String Value Provider Constructor should throw an error for non-string format:
     AssertionError: expected [Function: wrapper] to throw error including 'The format of options is invalid. Current format: non-string' but got 'The format of options is invalid. Current format: 1'
      at Context.<anonymous> (test/worker/workload/declarative/value-providers/formatted-string-value-provider.test.js:49:37)

@davidkel davidkel linked an issue Jul 4, 2022 that may be closed by this pull request
4 tasks
@davidkel
Copy link
Contributor

closing as stale now

@davidkel davidkel closed this Oct 22, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Make logging consistent
3 participants