ozone
ozone copied to clipboard
apache
HDDS-7280. New test addition to TestOMBucketCreateRequestWithFSO
What changes were proposed in this pull request?
While looking into the bug fixed in this PR, went over the tests for the bucket create request. The existing tests were all using a ClientVersion of value 0 which led to only covering the first code block here. In this patch, I added a new test method which uses the latest ClientVersion and covers this code block here. For consistency, the old tests will continue using ClientVersion.DEFAULT_VERSION as explained in this comment. These changes were part of #3750 but were deemed irrelevant and moved to this new PR.
What is the link to the Apache JIRA
https://issues.apache.org/jira/browse/HDDS-7280
How was this patch tested?
All unit tests are passing.
@kerneltime @errose28 That's the new PR for the test changes from https://github.com/apache/ozone/pull/3750.
@errose28 The point is not the ClientVersion but the BucketLayout used due to the version. All the existing tests are covering this code block. The purpose of the test changes is to have a setup that covers the rest of this method.
Can we just update OMRequestTestUtils to pass the latest client version in createBucketRequest and createBucketReqFSO without the other changes? It looks like the tests calling these methods still pass as expected with just this one change.
If you take a look at OMBucketCreateRequest.getDefaultBucketLayout() you will see that these test methods are expecting to default to BucketLayout.LEGACY due to the old ClientVersion. Right now, the system default is also LEGACY which makes all the tests pass. What if the system default changes? Then all the tests, which will be using the latest ClientVersion will get the bucket layout from the config which will be OBS or FSO but still expect to get LEGACY, like here.
If we change the ClientVersion to use the latest, someone would have to fix the tests later.
Thanks for the updates @xBis7. Current changes LGTM, can you just check if the test failures are related?
If we change the ClientVersion to use the latest, someone would have to fix the tests later.
That will be me 😄 In #3966 there's many other tests that currently fail when you change the default OM bucket layout so this will just be another one I can update.
can you just check if the test failures are related?
@errose28 Both of them passed on my fork's workflow. Check here.
Yup looks good. Datanode failure should not be related to this change. Key deleting test failure has failed in the same way on master before. Let me retrigger the tests and I can merge once it is green.