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

HDDS-1745. Add integration test for createDirectory for OM HA #1304

Closed
wants to merge 3 commits into from

Conversation

amaliujia
Copy link
Contributor

What changes were proposed in this pull request?

Add an integration test for createDirectory which is implemented as part of HDDS-1730 for OM HA.

What is the link to the Apache JIRA

https://issues.apache.org/jira/browse/HDDS-1745

How was this patch tested?

Integration Test.

@amaliujia
Copy link
Contributor Author

amaliujia commented Aug 8, 2020

@bharatviswa504

Can you give me some suggestions on this PR:

  1. Am I writing the integration test in the expected way?
  2. Any suggestion that what kind of cases in your mind that this PR should test?
    - Right now I am just testing a normal successful CreateDirectory call.

Copy link
Contributor

@bharatviswa504 bharatviswa504 left a comment

Choose a reason for hiding this comment

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

I have a comment on test case.

objectStore.createVolume(volumeName);
objectStore.getVolume(volumeName).createBucket(bucketName);

OMRequest request = OMRequest.newBuilder().setCreateDirectoryRequest(
Copy link
Contributor

Choose a reason for hiding this comment

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

We do not need this kind of tests, they are covered in UT.

We can use createDirectory API and test create Directory functionality.

Test cases can be:

  1. single level path ('/dir)
  2. Multi-level ('/dir1/dir2')
  3. Few parents exist in multi-level path
  4. Already a directory exists with the same name

any other cases you want to add :)

Copy link
Contributor Author

@amaliujia amaliujia Aug 11, 2020

Choose a reason for hiding this comment

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

@bharatviswa504 thank you!

I was confused on which createDirectory API it is. Can you paste a link here so I have better understanding on the context?

Copy link
Contributor

Choose a reason for hiding this comment

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

Copy link
Contributor Author

@amaliujia amaliujia Aug 11, 2020

Choose a reason for hiding this comment

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

Ah interesting..

If I run

    bucket.createDirectory("/dir1");
    bucket.createDirectory("/dir1");

The test will still pass without a complain. I expect an exception should be thrown.

I will check the internal of OM to see how does CreateDirectory API work, and see if there is anything can be improved.

Copy link
Contributor Author

@amaliujia amaliujia Aug 12, 2020

Choose a reason for hiding this comment

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

@bharatviswa504

I am currently have a problem to verify that a directory is created successfully.

What is the API to retrieve the information of directory? Seems that it is not considered as a key or a file so either getKey and getFileStatus does not work.

Basically I can do

bucket.createDirectory("/dir1"); then I need to do a bucket.getDirectoryStatus.

I also tried another direction: after a createDirectory call, try to put a file to that directory and then read that file to justify that createDirectory succeed. Later I realized this also does not work because without createDirectory, the create file call will always work even for multiple level directory (e.g. /dir1/dir2/dir3/file).

@adoroszlai
Copy link
Contributor

/pending

Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

Marking this issue as un-mergeable as requested.

Please use /ready comment when it's resolved.

Please note that the PR will be closed after 21 days of inactivity from now. (But can be re-opened anytime later...)

/pending

@amaliujia amaliujia closed this Jan 29, 2021
@amaliujia amaliujia deleted the HDDS-1745 branch January 29, 2021 18:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
3 participants