Skip to content
This repository has been archived by the owner on May 12, 2021. It is now read-only.

(TWILL-177) Make ZKDiscoveryService AutoCloseable #8

Closed
wants to merge 1 commit into from

Conversation

chtyim
Copy link
Member

@chtyim chtyim commented Aug 29, 2016

  • Release ZK watches when closed


@Before
public void beforeTest() {
LOG.info("Before test {}", testName.getMethodName());
Copy link
Contributor

Choose a reason for hiding this comment

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

this was just for investigation? Can be removed now?

Copy link
Member Author

Choose a reason for hiding this comment

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

Why? It helps identify which parts in the log is for which test.

Copy link
Contributor

Choose a reason for hiding this comment

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

Wouldn't JUnit already log the test method before it is executed?

Copy link
Member Author

Choose a reason for hiding this comment

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

No, it won't

@chtyim
Copy link
Member Author

chtyim commented Sep 12, 2016

Rebased on master and resolved conflicts.

import org.junit.Before;
import org.junit.Rule;
import org.junit.rules.ExternalResource;
import org.junit.rules.TestName;
Copy link
Contributor

Choose a reason for hiding this comment

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

unused imports?

Copy link
Member Author

Choose a reason for hiding this comment

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

fixed

@albertshau
Copy link
Contributor

one minor comment, otherwise lgtm

- Release ZK watches when closed
- Fixed a race condition in ZKDiscoveryServiceTest
- Fix the easy to fail InitializeFailTestRun
  - Match with an error message emitted from AM instead of from the container.
    - It’s possible that the one from the container don’t have chance to
      send out before it gets killed.
@chtyim
Copy link
Member Author

chtyim commented Sep 13, 2016

Squashed commits.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants