Skip to content

Fix flaky test#127

Merged
rsandell merged 1 commit intojenkinsci:masterfrom
jtnord:deflate-tests
Apr 28, 2023
Merged

Fix flaky test#127
rsandell merged 1 commit intojenkinsci:masterfrom
jtnord:deflate-tests

Conversation

@jtnord
Copy link
Member

@jtnord jtnord commented Apr 28, 2023

observed several failures due to

java.net.BindException: Address already in use
	at java.base/sun.nio.ch.Net.bind0(Native Method)
	at java.base/sun.nio.ch.Net.bind(Net.java:459)
	at java.base/sun.nio.ch.Net.bind(Net.java:448)
	at java.base/sun.nio.ch.AsynchronousServerSocketChannelImpl.bind(AsynchronousServerSocketChannelImpl.java:164)
	at org.apache.sshd.common.io.nio2.Nio2Acceptor.bind(Nio2Acceptor.java:83)
	at org.apache.sshd.common.io.nio2.Nio2Acceptor.bind(Nio2Acceptor.java:173)
	at org.apache.sshd.server.SshServer.start(SshServer.java:331)
	at com.cloudbees.jenkins.plugins.sshagent.SSHAgentBase.startMockSSHServer(SSHAgentBase.java:171)

rather than obtain a port ant t1 and then use it later at t2 just get the SSH server to use a port and then find out which port is actually being used.

  • Make sure you are opening from a topic/feature/bugfix branch (right side) and not your main branch!
  • Ensure that the pull request title represents the desired changelog entry
  • Please describe what you did
  • Link to relevant issues in GitHub or Jira
  • Link to relevant pull requests, esp. upstream and downstream changes
  • Ensure you have provided tests - that demonstrates feature works or fixes the issue

observed several failures due to

```
java.net.BindException: Address already in use
	at java.base/sun.nio.ch.Net.bind0(Native Method)
	at java.base/sun.nio.ch.Net.bind(Net.java:459)
	at java.base/sun.nio.ch.Net.bind(Net.java:448)
	at java.base/sun.nio.ch.AsynchronousServerSocketChannelImpl.bind(AsynchronousServerSocketChannelImpl.java:164)
	at org.apache.sshd.common.io.nio2.Nio2Acceptor.bind(Nio2Acceptor.java:83)
	at org.apache.sshd.common.io.nio2.Nio2Acceptor.bind(Nio2Acceptor.java:173)
	at org.apache.sshd.server.SshServer.start(SshServer.java:331)
	at com.cloudbees.jenkins.plugins.sshagent.SSHAgentBase.startMockSSHServer(SSHAgentBase.java:171)
```

rather than obtain a port ant t1 and then use it later at t2 just get the SSH server to use a port and then find out which port is actually being used.
Comment on lines +32 to +33
@Rule
public TemporaryFolder temp = new TemporaryFolder();
Copy link
Member Author

Choose a reason for hiding this comment

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

the same file was being used, which if using mutliple tests in parallel could lead to some funky timing issues also.

Copy link
Member

Choose a reason for hiding this comment

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

Will JUnit actually apply this rule since it isn't a test suite?

Copy link
Member Author

Choose a reason for hiding this comment

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

its a base class of the other tests so it should. the test pass :)

Copy link
Member

Choose a reason for hiding this comment

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

Meaning will the temp files created with it actually be cleaned up?

Copy link
Member Author

Choose a reason for hiding this comment

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

yup!
Note that it was not cleaned up prior to this - one would always dangle.

@jtnord jtnord marked this pull request as ready for review April 28, 2023 11:31
File hostKey = temp.newFile();
sshd = SshServer.setUpDefaultServer();
sshd.setPort(getValidPort());
sshd.setPort(0);
Copy link
Member Author

Choose a reason for hiding this comment

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

get the SSH server to start on a random port - so that it is guaranteed to be available.

*
* @return int with a valid port number
*/
private int getValidPort() throws IOException {
Copy link
Member Author

Choose a reason for hiding this comment

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

the port may be free immediately after it was obtained - but may have been used by something else before it was used by the SSH server.

*/
public int getAssignedPort() {
return assignedPort;
return sshd.getPort();
Copy link
Member Author

Choose a reason for hiding this comment

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

ask the ssh server for its port - it knows what it is!


protected void startMockSSHServer() throws Exception {
File hostKey = new File(System.getProperty("java.io.tmpdir") + "/key.ser");
hostKey.delete(); // do not carry from test to test
Copy link
Member Author

Choose a reason for hiding this comment

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

because this was using the same file accross tests and deleting it if it existed!
(running multiple tests in parallel!)

@rsandell rsandell merged commit 878b53c into jenkinsci:master Apr 28, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants