Skip to content

remove some of thread.sleeps in test cases #1786

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.

Sign up for GitHub

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

Merged
merged 4 commits into from
Jul 21, 2021
Merged

remove some of thread.sleeps in test cases #1786

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
011b45b
remove some of thread.sleeps in test cases
karthikkondapally Jul 17, 2021
6519c77
fix formatting issue
karthikkondapally Jul 17, 2021
6c07bfa
moved semaphores to countdownlatch
karthikkondapally Jul 19, 2021
b8d8804
undo the wrong change
karthikkondapally Jul 20, 2021
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
16 changes: 13 additions & 3 deletions extended/src/test/java/io/kubernetes/client/extended/leaderelection/LeaderElectionTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -285,16 +285,26 @@ public void testLeaderElectionCaptureException() throws ApiException, Interrupte
leaderElectionConfig.setLeaseDuration(Duration.ofMillis(1000));
leaderElectionConfig.setRetryPeriod(Duration.ofMillis(200));
leaderElectionConfig.setRenewDeadline(Duration.ofMillis(700));
final Semaphore sem = new Semaphore(1);
LeaderElector leaderElector =
new LeaderElector(leaderElectionConfig, (t) -> actualException.set(t));
new LeaderElector(
leaderElectionConfig,
(t) -> {
actualException.set(t);
sem.release();
});

ExecutorService leaderElectionWorker = Executors.newFixedThreadPool(1);
sem.acquire();
leaderElectionWorker.submit(
() -> {
leaderElector.run(() -> {}, () -> {});
});
// TODO: Remove this sleep
Thread.sleep(Duration.ofSeconds(2).toMillis());
while (!sem.tryAcquire()) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Is there a reason for this loop? I would remove all this code and just do:

sem.acquire() like you did above.

Copy link
Contributor Author

@karthikkondapally karthikkondapally Jul 19, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

i wanted to print message on why we are waiting..
but i have changed semaphores to countdown latch, as i thought it would be better than sempahore in these situations

System.out.println(
"waiting for leaderElectionWorker to throw exception in LeaderElectionTest::testLeaderElectionCaptureException");
}

assertEquals(expectedException, actualException.get().getCause());
}

Expand Down
18 changes: 12 additions & 6 deletions util/src/test/java/io/kubernetes/client/ExecTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@
import java.io.InputStream;
import java.io.OutputStream;
import java.nio.charset.StandardCharsets;
import java.util.concurrent.Semaphore;
import org.junit.Before;
import org.junit.Rule;
import org.junit.Test;
Expand Down Expand Up @@ -82,7 +83,7 @@ public static InputStream makeStream(byte[] prefix, byte[] data) {
return new ByteArrayInputStream(out);
}

public static Thread asyncCopy(final InputStream is, final OutputStream os) {
public static Thread asyncCopy(final InputStream is, final OutputStream os, Semaphore sem) {
Thread t =
new Thread(
new Runnable() {
Expand All @@ -91,6 +92,8 @@ public void run() {
Streams.copy(is, os);
} catch (IOException ex) {
ex.printStackTrace();
} finally {
sem.release();
}
}
});
Expand All @@ -110,14 +113,17 @@ public void testExecProcess() throws IOException, InterruptedException {

final ByteArrayOutputStream stdout = new ByteArrayOutputStream();
final ByteArrayOutputStream stderr = new ByteArrayOutputStream();

Thread t1 = asyncCopy(process.getInputStream(), stdout);
Thread t2 = asyncCopy(process.getErrorStream(), stderr);
final Semaphore sem = new Semaphore(2);
sem.acquire(2);
Thread t1 = asyncCopy(process.getInputStream(), stdout, sem);
Thread t2 = asyncCopy(process.getErrorStream(), stderr, sem);

process.getHandler().bytesMessage(makeStream(3, OUTPUT_EXIT0.getBytes(StandardCharsets.UTF_8)));
// TODO: Fix this asap!
Thread.sleep(1000);

while (!sem.tryAcquire(2)) {
Copy link
Contributor

Choose a reason for hiding this comment

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

As above, why not just sem.acquire(2)?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

wanted to print message on why we are waiting..
but i have changed semaphores to countdown latch, as i thought it would be better than sempahore in these situations

System.out.println(
"waiting for async Copy task to be completed in ExecTest::testExecProcess");
}
process.destroy();

assertEquals(msgData, stdout.toString());
Expand Down