Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
HDDS-8450. Dedicated acceptance test suite for s3a #6458
HDDS-8450. Dedicated acceptance test suite for s3a #6458
Changes from all commits
4e645cd
f0fc477
a34ecd9
5e0146d
add9ce2
3ffb95c
288fcd2
da2dc75
45be98e
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It might be nice to also test for virtual host style pattern to uncover some AWS Java SDK problems with it (e.g. HDDS-7035) since our S3G implementation is built on top of path style pattern assumption (i.e. S3G needs to use
VirtualHostStyleFilter
to convert virtual host style -> path style).This can be addressed in future patches if needed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe add a comment or rename this method to clarify that this method only applies to tests run with Robot.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Generalized
s3a
-specific condition to check acceptance test type (robot
ormaven
). We may be adding more Maven-based tests in the future.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Although the overall job passes, there is still a failure because
generate_report
is being called (not sure why this doesn't fail the s3a job):Looks like
OZONE_ACCEPTANCE_TEST_TYPE
is not exported inacceptance.sh
and not visible in this subshell.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Side note, will this also be a problem if devs run
test-all.sh
manually to run all the tests locally?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for pointing out the problem with
OZONE_ACCEPTANCE_TEST_TYPE
.When running
test-all.sh
directly, installing Robot is the developer's responsibility (it was already required before this change).There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Right, my question is more generally: what is the expected way to run all acceptance tests locally and manually? Ideally running
test-all.sh
(or maybeacceptance.sh
) should kick off all the acceptance tests locally. After this change, running either of these will only run the robot tests, unless the user knows which variables to export to their shell before running.Maybe that's ok for now and we can handle that in a follow up task, since there's probably not many people running all the tests this way.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actually, they will also try to run
s3a
tests, but that part will fail, since Hadoop sources will be missing.We can avoid that by further separating the two kinds of tests.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm ok to handle this in a follow up task if you want. It works fine in CI and I don't think many people are running the tests this way.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Slightly changed
s3a-test.sh
to not fail, rather skip the test ifHADOOP_AWS_DIR
is missing. This way current behavior is retained when runningacceptance.sh
ortest-all.sh
without any test filters.