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

Test for presence of ack result message and simplify ProcessControllerAckResult API #7816

Merged
merged 1 commit into from
Dec 23, 2022
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Test for presence of ack result message and simplify ProcessControlle…
…rAckResult API.
cmnbroad committed May 2, 2022
commit c7832fb01fa6d7511e1d4999c55d6e59d42b15af
2 changes: 2 additions & 0 deletions Dockerfile
Original file line number Diff line number Diff line change
@@ -45,6 +45,8 @@ WORKDIR /gatk

# Create a simple unit test runner
ENV CI true
# "export GATK_DOCKER_CONTAINER=true" is used to allow tests to determine when the're running on the docker
# (some negative python tests use this to throw skip exceptions). See GATKBaseTest::isGATKDockerContainer.
RUN echo "source activate gatk" > /root/run_unit_tests.sh && \
echo "export GATK_DOCKER_CONTAINER=true" >> /root/run_unit_tests.sh && \
echo "export TEST_JAR=\$( find /jars -name \"gatk*test.jar\" )" >> /root/run_unit_tests.sh && \
Original file line number Diff line number Diff line change
@@ -4,7 +4,8 @@

/**
* Command acknowledgements that are returned from a process managed by StreamingProcessController.
* Ack results can be positive, negative, or negative with a message.
* Ack results can be positive, negative, or negative with a message. Positive acks never have a messsage,
* negative acks may optionally have a message.
*/
public class ProcessControllerAckResult {

@@ -46,18 +47,7 @@ public boolean isPositiveAck() {
* @return true if this ack is negative and includes a message
*/
public boolean hasMessage() {
return !isPositiveAck() && !message.isEmpty();
}

/**
* @return A (possibly empty) String with any message sent from the remote process.
* Only defined for negative acknowledgements {@link #hasMessage()}.
*/
public String getNegativeACKMessage() {
if (isPositiveAck()) {
throw new GATKException("Can only retrieve messages for negative acknowledgements");
}
return message;
return !isPositiveAck() && message != null && !message.isEmpty();
}

/**
@@ -67,7 +57,7 @@ public String getDisplayMessage() {
if (isPositiveAck()) {
return ACK_LOG_MESSAGE;
} else if (hasMessage()) {
return String.format("%s: %s", NCK_WITH_MESSAGE_LOG_MESSAGE, getNegativeACKMessage());
return String.format("%s: %s", NCK_WITH_MESSAGE_LOG_MESSAGE, message);
} else {
return NCK_LOG_MESSAGE;
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,51 @@
package org.broadinstitute.hellbender.utils.runtime;

import org.broadinstitute.hellbender.GATKBaseTest;
import org.testng.Assert;
import org.testng.annotations.DataProvider;
import org.testng.annotations.Test;

public class ProcessControllerAckResultUnitTest extends GATKBaseTest {

@DataProvider(name="ackResultTests")
private Object[][] getAckResultTests() {
return new Object[][] {
// ack result,
// isPositiveAck, hasMessage, getDisplayMessage prefix
{ new ProcessControllerAckResult(true),
true, false, StreamingToolConstants.STREAMING_ACK_MESSAGE },
{ new ProcessControllerAckResult(false),
false, false, StreamingToolConstants.STREAMING_NCK_MESSAGE },
{ new ProcessControllerAckResult("some message"),
false, true, StreamingToolConstants.STREAMING_NCK_WITH_MESSAGE_MESSAGE },
};
}

@Test(dataProvider = "ackResultTests")
private void testHasMessage(
final ProcessControllerAckResult ackResult,
final boolean unusedIsPositiveAck,
final boolean hasMessage,
final String unusedGetDisplayMessagePrefix) {
Assert.assertEquals(ackResult.hasMessage(), hasMessage);
}

@Test(dataProvider = "ackResultTests")
private void testIsPositiveAck(
final ProcessControllerAckResult ackResult,
final boolean isPositiveAck,
final boolean unusedHasMessage,
final String unusedGetDisplayMessagePrefix) {
Assert.assertEquals(ackResult.isPositiveAck(), isPositiveAck);
}

@Test(dataProvider = "ackResultTests")
private void testGetDisplayMessage(
final ProcessControllerAckResult ackResult,
final boolean unusedIsPositiveAck,
final boolean unusedHasMessage,
final String getDisplayMessagePrefix) {
Assert.assertTrue(ackResult.getDisplayMessage().startsWith(getDisplayMessagePrefix));
}

}