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

test(connector-besu): migrate get-past-logs-endpoint to Jest #3608

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

Akshitha1020
Copy link

@Akshitha1020 Akshitha1020 commented Oct 30, 2024

Commit to be reviewed

test(connector-besu): migrate get-past-logs-endpoint to Jest

 Primary Changes
 ------------------
 1. Updated the migrate get-past-logs-endpoint test 
     to use Jest
 2. Updated the ci.yaml TAPE_TEST_PATTERN to
     incorporate the same

Fixes #3505

Pull Request Requirements

  • Rebased onto upstream/main branch and squashed into single commit to help maintainers review it more efficient and to avoid spaghetti git commit graphs that obfuscate which commit did exactly what change, when and, why.
  • Have git sign off at the end of commit message to avoid being marked red. You can add -s flag when using git commit command. You may refer to this link for more information.
  • Follow the Commit Linting specification. You may refer to this link for more information.

Character Limit

  • Pull Request Title and Commit Subject must not exceed 72 characters (including spaces and special characters).
  • Commit Message per line must not exceed 80 characters (including spaces and special characters).

A Must Read for Beginners
For rebasing and squashing, here's a must read guide for beginners.

Copy link
Contributor

@petermetz petermetz left a comment

Choose a reason for hiding this comment

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

@Akshitha1020 Thank you! LGTM

Copy link
Contributor

@petermetz petermetz left a comment

Choose a reason for hiding this comment

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

@Akshitha1020 Please fix the lint CI check that failed with the message pasted below:

Error: yarn lint script produced version control side-effects: source files have been changed by it that are otherwise are under version control. This means (99% of the time) that you need to run the yarn lint script locally and then include the changes it makes in your own commit when submitting your pull request.

(you can run the linter locally by doing yarn lint on the command line)

@Akshitha1020
Copy link
Author

Hi @petermetz, tried running yarn lint locally, but no issues are found, please find the screenshot attached. Could you please help me resolve this.
image

@jagpreetsinghsasan
Copy link
Contributor

@Akshitha1020 after running yarn lint you will see some files modified. You have to commit those changes (Please make sure to squash merge all the commits into one single commit before pushing).

In this PR, the lint failed here packages/cactus-test-plugin-ledger-connector-besu/src/test/typescript/integration/plugin-validator-besu/get-past-logs-endpoint.test.ts line 92 (You need to add a space after IPluginLedgerConnectorBesuOptions and before =

@jagpreetsinghsasan
Copy link
Contributor

Also, to fix the PR - Commit Parity failing check, please update your commit message to this (I have updated the PR Desciption accordingly)

test(connector-besu): migrate get-past-logs-endpoint to Jest 

     Primary Changes
     ------------------
     1. Updated the migrate get-past-logs-endpoint test 
         to use Jest
     2. Updated the ci.yaml TAPE_TEST_PATTERN to
         incorporate the same

Fixes #3505

@Akshitha1020 Akshitha1020 force-pushed the test-jest-migrate-connector-besu branch 2 times, most recently from 8508fd6 to cd3b61a Compare November 5, 2024 07:52
@Akshitha1020
Copy link
Author

Hi @jagpreetsinghsasan, I have done the changes as you mentioned, please re-review and let me know.

@jagpreetsinghsasan
Copy link
Contributor

@Akshitha1020 all good from my past review comment. There is a merge conflict, can you please resolve that as well?
image

@petermetz
Copy link
Contributor

@Akshitha1020 If you are end up struggling with the git operations needed to resolve the merge conflicts just let me know and I can help (You'll just need to invite me as a collaborator to your fork so that I have permission to push to your PR branch directly)

@Akshitha1020 Akshitha1020 force-pushed the test-jest-migrate-connector-besu branch from 51cbdf6 to f93bee4 Compare November 11, 2024 06:30
@Akshitha1020
Copy link
Author

Hi @petermetz, resolved the conflicts but on squashing the commits again it is showing conflicts. I have invited you to be a collaborator to my fork, please accept.

@petermetz
Copy link
Contributor

Hi @petermetz, resolved the conflicts but on squashing the commits again it is showing conflicts. I have invited you to be a collaborator to my fork, please accept.

@Akshitha1020 Okay, no worries! I accepted the invite just now, I'll push the changes soon.

     Primary Changes
     ------------------
     1. Updated the migrate get-past-logs-endpoint test
         to use Jest
     2. Updated the ci.yaml TAPE_TEST_PATTERN to
         incorporate the same

Fixes hyperledger-cacti#3505

Co-authored-by: Peter Somogyvari <peter.somogyvari@accenture.com>

Signed-off-by: Akshitha1020 <159411458+Akshitha1020@users.noreply.github.com>
Signed-off-by: Peter Somogyvari <peter.somogyvari@accenture.com>
@petermetz petermetz force-pushed the test-jest-migrate-connector-besu branch from f93bee4 to de6d608 Compare November 11, 2024 18:49
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.

test(connector-besu): migrate get-past-logs test cases from Tap to Jest
3 participants