fix test failing because of timeout that aren't testing timing #375
Conversation
scottf
changed the title
| } | ||
|
|
||
| handler.waitForStatusChange(400, TimeUnit.MILLISECONDS); | ||
| try { nc.flush(Duration.ofMillis(250)); } catch (Exception exp) { /* ignored */ } |
There was a problem hiding this comment.
Increased flush wait from 100 to 250 as it was flapping here. The test is about something else so this just needs to always work.
| assertTrue(one == 10, "always got one"); | ||
| assertTrue(two == 0, "never got two"); | ||
| assertEquals(one, 10, "always got one"); | ||
| assertEquals(two, 0, "never got two"); |
There was a problem hiding this comment.
nitty for sure, but again how the assertions were designed.
There was a problem hiding this comment.
I see linter complaints about these... thanks.
| } catch (Exception exp) { | ||
|
|
||
| } | ||
| sleep(5000); // No server at this point, let it fail and try to start over |
There was a problem hiding this comment.
sleep macro just cleaner.
There was a problem hiding this comment.
could be a bit more deterministic and use options.getConnectionTimeout() + 100 in case the default timeout changes.
| } | ||
| } | ||
| } | ||
|
|
||
| @Test | ||
| public void testConnectExceptionHasURLS() throws IOException, InterruptedException { | ||
| public void testConnectExceptionHasURLS() { |
There was a problem hiding this comment.
exception was never thrown
|
|
||
| handler.waitForStatusChange(1000, TimeUnit.MILLISECONDS); |
There was a problem hiding this comment.
After I fixed that one, this one flapped.
|
|
||
| nc = handler.getConnection(); // will be disconnected, but should be there | ||
| assertNotNull(nc); | ||
|
|
||
| handler.prepForStatusChange(Events.RECONNECTED); | ||
| try (NatsTestServer ts = new NatsTestServer(port, false)) { | ||
| handler.waitForStatusChange(5, TimeUnit.SECONDS); | ||
| assertTrue(Connection.Status.CONNECTED == nc.getStatus(), "Connected Status"); | ||
| waitThenAssertConnected(nc, handler); |
There was a problem hiding this comment.
This macro fixed the flappers that didn't allow enough time for the connection. With the macro it's consistent. Only used in tests where they are not testing the connection itself, but using the connection to test something. This version of the macro waits up to 5 seconds. There is another version that you can specify the wait.
| } | ||
| assertTrue(Connection.Status.CONNECTED == nc.getStatus(), "Connected Status"); | ||
|
|
||
| sleep(1000); |
There was a problem hiding this comment.
more flapping. again not the point of the test
| d.subscribe("dispatchSubject"); | ||
|
|
||
| handler.prepForStatusChange(Events.RECONNECTED); | ||
|
|
||
| try (NatsTestServer ts = new NatsTestServer(port, false)) { | ||
| handler.waitForStatusChange(400, TimeUnit.MILLISECONDS); | ||
| assertTrue(Connection.Status.CONNECTED == nc.getStatus(), "Connected Status"); | ||
| waitThenAssertConnected(nc, handler); |
There was a problem hiding this comment.
not enough time to ensure connect. use macro since this not the point of the test
| } finally { | ||
| nc.close(); | ||
| assertTrue(Connection.Status.CLOSED == nc.getStatus(), "Closed Status"); | ||
| closeThenAssertClosed(nc); |
There was a problem hiding this comment.
I noticed it was the same thing over and over with no variation. "Macros" aren't really a java thing, just something I like. I think they add description to the tests and simplify routine scaffolding code.
| handler.prepForStatusChange(Events.CLOSED); | ||
| } | ||
|
|
||
| flushAndWait(nc, handler); | ||
| assertTrue(Connection.Status.CLOSED == nc.getStatus(), "Closed Status"); | ||
| assertSame(Connection.Status.CLOSED, nc.getStatus(), "Closed Status"); |
There was a problem hiding this comment.
assertSame is recommended for enum, even though it does the exact same thing. Intellij called it "simplify"
These test are flappers, failing because of timeout. But they are not testing timing, they are testing state, so timeout should not matter.
While I was in these files... nit fixed to use assertSame when testing connection status / enums