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

Add more test for tx observers #1617

Conversation

tunedev
Copy link
Contributor

@tunedev tunedev commented Sep 9, 2024

Checklist

  • A link to the issue/user story that the pull request relates to
  • How to recreate the problem without the fix
  • Design of the fix
  • How to prove that the fix works
  • Automated tests that prove the fix keeps on working
  • Documentation - any JSDoc, website, or Stackoverflow answers?

Issue/User story

Partially addresses #1606

Steps to Reproduce

Existing issues

Design of the fix

Validation of the fix

After this PR: Running the tests in caliper-core the below listed %stmts in the code coverage report should tally with the below listed

  • caliper-core/lib/worker/tx-observers: 93.06
  • caliper-core/lib/worker/tx-observers/internal-tx-observer.js: 100
  • caliper-core/lib/worker/tx-observers/logging-tx-observer.js: 100
  • caliper-core/lib/worker/tx-observers/prometheus-tx-observer.js : 92.06
  • caliper-core/lib/worker/tx-observers/tx-observer-dispatch.js : 92.59

Screenshot from 2024-09-09 13-36-44

Before the PR: Running the tests in caliper-core the below listed %stmts in the code coverage report the below listed was what was there before

  • caliper-core/lib/worker/tx-observers: 68.82
  • caliper-core/lib/worker/tx-observers/internal-tx-observer.js: 26.08
  • caliper-core/lib/worker/tx-observers/logging-tx-observer.js: 0
  • caliper-core/lib/worker/tx-observers/prometheus-tx-observer.js : 77.77
  • caliper-core/lib/worker/tx-observers/tx-observer-dispatch.js : 19.23

Screenshot from 2024-09-09 13-40-12

Automated Tests

What documentation has been provided for this pull request

@tunedev tunedev requested a review from a team September 9, 2024 12:50
@aklenik aklenik added the mentorship-2024-testing Part of the testing project of Hyperledger Mentorship 2024 label Sep 10, 2024
@aklenik aklenik self-assigned this Sep 10, 2024
Copy link
Contributor

@davidkel davidkel left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unfortunately I think that there is too much in this PR. I think it would be easier to review if we have much smaller PRs and each PR should focus on a single unit (ie .js file under test). If we do it this way I think it will be easier to focus and also we can get code merged more quicky. Please could you split the PR into multiple ones

@tunedev tunedev force-pushed the add-more-test-for-tx-observers branch 3 times, most recently from da53eeb to d054b8e Compare September 11, 2024 08:28
…s folder:

internal-tx-observers
logging-tx-observers
prometheus-tx-observer
tx-observer-dispatch

Signed-off-by: Babatunde Sanusi <swisskid95@gmail.com>
@tunedev tunedev force-pushed the add-more-test-for-tx-observers branch from d054b8e to dcc156f Compare September 11, 2024 08:43
@tunedev tunedev closed this Sep 11, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
mentorship-2024-testing Part of the testing project of Hyperledger Mentorship 2024
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants