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(test-geth-ledger): add test for test-geth-ledger #2588

Closed
wants to merge 3 commits into from

Conversation

rwat17
Copy link
Contributor

@rwat17 rwat17 commented Jul 28, 2023

  • add tests for geth-test-ledger
  • fix small errors in GethTestLedger class

Closes: #2579
Depends on: #2577

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.

@rwat17 In addition to my above comment, could you please

  1. Rename the .json file so that it is an exact match to the .sol file but with a different extenssion.
  2. Obtain the JSON file by compiling it with the solidity VSCode extension which adds the contractName attribute (among other things) automatically as useful metadata that we sometimes use programmatically in test cases for reducing the number of magic strings the tests are depending on to function.

As an example of such .json file please take a look at this one: packages/cactus-plugin-htlc-eth-besu-erc20/src/main/solidity/contracts/HashedTimeLockContract.json

For the specific VSCode extension IDs please see the recommended extensions file under .vscode/extensions.json in there you'll see "juanblanco.solidity"

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.

@rwat17 Please take a look at my commit and squash it so that there's (in the end) only one commit in the PR (after the parent PR has been merged)
Then pass it back for review!

https://github.com/hyperledger/cacti/pull/2588/commits/696621bbd2003f6b47f3e721e890db8630778e1e

@rwat17 rwat17 force-pushed the geth-ledger-tests branch 2 times, most recently from 4facda4 to e9d6694 Compare August 17, 2023 13:17
outSH and others added 3 commits August 17, 2023 16:28
-add tests for geth-test-ledger
-fix small errors in GethTestLedger class

Closes: hyperledger-cacti#2579
Depends on: hyperledger-cacti#2577

Signed-off-by: Tomasz Awramski <[email protected]
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.

LGTM

Copy link
Contributor

@izuru0 izuru0 left a comment

Choose a reason for hiding this comment

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

LGTM

@petermetz petermetz self-requested a review August 31, 2023 19:27
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.

@rwat17 Please

  1. Fix the DCO check
  2. Fix the dependency declaration syntax (the robot is pretty simple minded unfortunately and won't understand Depends on: #2577 only Depends on #2577 (notice the colon difference) - this is important so that we don't accidentally merge the PR before the parent have been merged
  3. Fix the build-dev check
  4. When everything else is ready and the parent PR(s) have been merged please do the usual rebase + squash onto upstream/main

Marking it as a change request so we don't accidentally merge while the dependencies are not met. (once done with the chores above just pass it back for review)

@petermetz
Copy link
Contributor

I'm guessing this one has gone stale too since @rwat17 moved on, closing for now. We can re-open it anytime as usual.

@petermetz petermetz closed this Oct 18, 2023
outSH added a commit to outSH/cactus that referenced this pull request Nov 24, 2023
- Add test suite for geth-test-ledger package.
- Add new test suit to CI
- It was initially proposed in hyperledger-cacti#2588, I've added some cleanups and improvements.

Closes: hyperledger-cacti#2579

Co-authored-by: Tomasz Awramski <[email protected]

Signed-off-by: Michal Bajer <[email protected]>
outSH added a commit to outSH/cactus that referenced this pull request Nov 24, 2023
- Add test suite for geth-test-ledger package.
- Add new test suit to CI
- It was initially proposed in hyperledger-cacti#2588, I've added some cleanups and improvements.

Closes: hyperledger-cacti#2579

Co-authored-by: Tomasz Awramski <[email protected]>

Signed-off-by: Michal Bajer <[email protected]>
outSH added a commit to outSH/cactus that referenced this pull request Dec 12, 2023
- Add test suite for geth-test-ledger package.
- Add new test suit to CI
- It was initially proposed in hyperledger-cacti#2588, I've added some cleanups and improvements.

Closes: hyperledger-cacti#2579

Co-authored-by: Tomasz Awramski <[email protected]>

Signed-off-by: Michal Bajer <[email protected]>
outSH added a commit to outSH/cactus that referenced this pull request Dec 18, 2023
- Add test suite for geth-test-ledger package.
- Add new test suit to CI
- It was initially proposed in hyperledger-cacti#2588, I've added some cleanups and improvements.

Closes: hyperledger-cacti#2579

Co-authored-by: Tomasz Awramski <[email protected]>

Signed-off-by: Michal Bajer <[email protected]>
outSH added a commit to outSH/cactus that referenced this pull request Dec 19, 2023
- Add test suite for geth-test-ledger package.
- Add new test suit to CI
- It was initially proposed in hyperledger-cacti#2588, I've added some cleanups and improvements.
- Run codegen, update missing deps, sort package.json,
    fix type in socketio-test-setup-helpers, to fix some CI.

Closes: hyperledger-cacti#2579

Co-authored-by: Tomasz Awramski <[email protected]>

Signed-off-by: Michal Bajer <[email protected]>
outSH added a commit to outSH/cactus that referenced this pull request Dec 19, 2023
- Add test suite for geth-test-ledger package.
- Add new test suit to CI
- It was initially proposed in hyperledger-cacti#2588, I've added some cleanups and improvements.
- Run codegen, update missing deps, sort package.json,
    fix type in socketio-test-setup-helpers, to fix some CI.

Closes: hyperledger-cacti#2579

Co-authored-by: Tomasz Awramski <[email protected]>

Signed-off-by: Michal Bajer <[email protected]>
outSH added a commit that referenced this pull request Dec 19, 2023
- Add test suite for geth-test-ledger package.
- Add new test suit to CI
- It was initially proposed in #2588, I've added some cleanups and improvements.
- Run codegen, update missing deps, sort package.json,
    fix type in socketio-test-setup-helpers, to fix some CI.

Closes: #2579

Co-authored-by: Tomasz Awramski <[email protected]>

Signed-off-by: Michal Bajer <[email protected]>
sandeepnRES pushed a commit to sandeepnRES/cacti that referenced this pull request Dec 21, 2023
- Add test suite for geth-test-ledger package.
- Add new test suit to CI
- It was initially proposed in hyperledger-cacti#2588, I've added some cleanups and improvements.
- Run codegen, update missing deps, sort package.json,
    fix type in socketio-test-setup-helpers, to fix some CI.

Closes: hyperledger-cacti#2579

Co-authored-by: Tomasz Awramski <[email protected]>

Signed-off-by: Michal Bajer <[email protected]>
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(geth-all-in-one): improve geth helper class tests coverage
4 participants