devmadhuu commented on PR #5217: URL: https://github.com/apache/ozone/pull/5217#issuecomment-1698447668
> > > Thanks for addressing review comments. > > > Overall changes are fine. > > > Few doubts and suggestions: > > > > > > 1. PR title is incorrect I think. It is supposed to be `HDDS-6646 Intermittent failure ....` > > > 2. Can you please share the workflow run where you run the test 10 times? I found [this](https://github.com/devmadhuu/ozone/actions/runs/5938962749/job/16109360981), but in last iteration test still failed. Also I would suggest to run only `testRenameToTrashEnabled` and have a green CI for it. > > > 3. I think [deleteRootDir](https://github.com/apache/ozone/blob/011de37b19ec874819ae5bbf726580ba5b0b7f93/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/fs/ozone/TestOzoneFileSystem.java#L721) is not needed here because it is deleted in [@After](https://github.com/apache/ozone/blob/011de37b19ec874819ae5bbf726580ba5b0b7f93/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/fs/ozone/TestOzoneFileSystem.java#L228). If you agree, remove this either as part of this PR or may be separate PR. > > > > > > @hemantk-12 , we can change PR title, however it's better to keep with JIRA title. > > I'll re-run the test in multiple runs once again and get green CI. > > I think we should keep deleteRootDir change as i observed that without this change, CI will not pass. Many other PR are blocked because of this > > 1. I didn't mean to remove Jira ID or Jira Title. `...` just mean the continuation. I meant `HDDS-6646. Intermittent failure in TestOzoneFileSystem#testRenameToTrashEnabled` not `Hdds 6646 - Intermittent failure in TestOzoneFileSystem#testRenameToTrashEnabled`. PR title is supposed to start with `HDDS-JiraID.` followed by PR or jIra title > 2. I am curious how `deleteRootDir()` calling solves the issue in test [testListStatusOnLargeDirectory](https://github.com/apache/ozone/blob/011de37b19ec874819ae5bbf726580ba5b0b7f93/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/fs/ozone/TestOzoneFileSystem.java#L720). Did you try to remove it and run? Do you have a run where it failed after it was removed? 1. I fixed the PR title. Thanks for pointing out. 2. `testListStatusOnLargeDirectory` test was failed like any other test due to deleteRootDIr cleanup. No changes being done except deleteRootDir, May be we can remove from PR description about this test. 3. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
