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

Create DoraWorkerServiceHandlerTest.java #18059

Merged
merged 15 commits into from
Sep 12, 2023
Merged

Create DoraWorkerServiceHandlerTest.java #18059

merged 15 commits into from
Sep 12, 2023

Conversation

thu-david
Copy link
Contributor

What changes are proposed in this pull request?
I have created a test and create a liststatus test for its function.

Why are the changes needed?
Please clarify why the changes are needed. For instance,

add a unit test for DoraWorkerClientServiceHandler.
Does this PR introduce any user facing changes?
No.


@Override
public void onCompleted() {
// do nothing
Copy link
Contributor

Choose a reason for hiding this comment

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

remove the comment

@Override
public void onCompleted() {
// do nothing
mResponses.sort(Comparator.comparing(MyStruct::getPath));
Copy link
Contributor

Choose a reason for hiding this comment

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

sort in the test instead of sorting in this test observer

TestStreamObserver responseObserver = new TestStreamObserver();
mServiceHandler.listStatus(mRequest, responseObserver);
List<MyStruct> responses = responseObserver.mResponses;
String[] expectedPaths = new String[] {rootPath + "/d1", rootPath + "/d1/d1",
Copy link
Contributor

Choose a reason for hiding this comment

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

PathUtils.concatPath

TestStreamObserver finalResponseObserver = responseObserver;
assertThrows(RuntimeException.class, () -> mServiceHandler.listStatus(mRequest,
finalResponseObserver));
}
Copy link
Contributor

Choose a reason for hiding this comment

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

  1. list a file
  2. list a directory that does not exist

}

@Test
public void testListStatus() throws IOException {
Copy link
Contributor

Choose a reason for hiding this comment

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

also test InAlluxioPercentage

@alluxio-bot
Copy link
Contributor

Automated checks report:

  • PR title follows the conventions: FAIL
    • The title of the PR does not pass all the checks. Please fix the following issues:
      • First word must be capitalized
  • Commits associated with Github account: PASS

Some checks failed. Please fix the reported issues and reply 'alluxio-bot, check this please' to re-run checks.

@thu-david thu-david changed the title create DoraWorkerServiceHandlerTest.java Create DoraWorkerServiceHandlerTest.java Aug 28, 2023
@alluxio-bot
Copy link
Contributor

Automated checks report:

  • PR title follows the conventions: PASS
  • Commits associated with Github account: PASS

All checks passed!

}

@Test
public void testListStatus() throws IOException {
Copy link
Contributor

Choose a reason for hiding this comment

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

can you create different tests instead?

fos.close();

AlluxioURI oldPath = TEST_FILE_URI;
for (int i = 1; i < 10; i++) {
Copy link
Contributor

Choose a reason for hiding this comment

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

we just need to test once

Copy link
Contributor

@elega elega left a comment

Choose a reason for hiding this comment

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

LGTM

@elega
Copy link
Contributor

elega commented Sep 12, 2023

alluxio-bot, merge this please.

@alluxio-bot alluxio-bot merged commit 3c41091 into Alluxio:main Sep 12, 2023
12 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type-code-quality code quality improvement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants