Skip to content

use tmp dir for --data-path in BesuCommand tests #9350

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.

Sign up for GitHub

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

Jump to bottom
Open
wants to merge 12 commits into
base: main
Choose a base branch
from
Open

use tmp dir for --data-path in BesuCommand tests #9350

wants to merge 12 commits into from

Conversation

@macfarla
Copy link
Contributor

@macfarla macfarla commented Oct 23, 2025 •
edited
Loading

Signed-off-by: Sally MacFarlane [email protected]

PR description

Modified abstract test class to add --data-path if not already present

  • exclude tests that use config file since this could contain data-path

Fixed Issue(s)

Fixes #9313

Thanks for sending a pull request! Have you done the following?

  • Checked out our contribution guidelines?
  • Considered documentation and added the doc-change-required label to this PR if updates are required.
  • Considered the changelog and included an update if required.
  • For database changes (e.g. KeyValueSegmentIdentifier) considered compatibility and performed forwards and backwards compatibility tests

Locally, you can run these tests to catch failures early:

  • spotless: ./gradlew spotlessApply
  • unit tests: ./gradlew build
  • acceptance tests: ./gradlew acceptanceTest
  • integration tests: ./gradlew integrationTest
  • reference tests: ./gradlew ethereum:referenceTests:referenceTests
  • hive tests: Engine or other RPCs modified?

@macfarla macfarla added flake 60% of the time it works 100% of the time. test labels Oct 23, 2025
siladu
siladu reviewed Oct 23, 2025
View reviewed changes
@macfarla
extract method
a049f47 
Signed-off-by: Sally MacFarlane <[email protected]>
@macfarla
notnull
5054c9f 
Signed-off-by: Sally MacFarlane <[email protected]>
@macfarla
final
0995240 
Signed-off-by: Sally MacFarlane <[email protected]>
@macfarla
don't do anything special for subcommands
67e5272 
Signed-off-by: Sally MacFarlane <[email protected]>
siladu
siladu reviewed Oct 23, 2025
View reviewed changes
app/src/test/java/org/hyperledger/besu/cli/CommandTestAbstract.java
final List<String> argsList = new ArrayList<>(Arrays.asList(args));

// If first arg starts with "-" or args is empty, it's likely main command
boolean isMainCommand = args.length == 0 || args[0].startsWith("-");
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we still need this?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

PublicKeySubCommandTest needs more work if we don't have this

Copy link
Contributor

Choose a reason for hiding this comment

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

I think the simplicity of applying to all subcommands here is worth making a special case out of PublicKeySubCommandTest

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@siladu siladu siladu left review comments

At least 1 approving review is required to merge this pull request.

Assignees

No one assigned

Labels

flake 60% of the time it works 100% of the time. test

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

flaky test: CLI Options test

2 participants

@macfarla @siladu