Upgrade parent pom & fix InputStream leak & fix tests #206
Conversation
|
Closing to relaunch CI. |
|
@darxriggs I'm merging this, the failures seem clearly related with infra issues. Thanks for your contribution! |
|
@darxriggs sorry but the test failures seem to keep happening. Could you have a look? Thanks! |
|
I rebased on master and triggered the tests again, no luck. I have seen these issues on ci.jenkins.io and also locally already multiple times. It's just a timing issue. On Windows the temporary files created during testing cannot be removed because they are still locked. There is nothing I can do about. |
|
It's weird because master works fine. Something must be happening. |
|
@jenkinsci/code-reviewers These CI issues on Windows are known. Does anybody of you know about a fix on ci.jenkins.io? |
|
@darxriggs usually those messages on Windows computers indicate that there is a file handle still open to the files being deleted. It may be a test defect. It may be a defect in the production code. It usually requires analysis by someone that can watch it fail on Windows. |
|
Yes, the CI failures likely indicate an actual problem: something holding a file lock. |
|
I am currently already looking at it and can reproduce it locally. |
|
I seems it is due to this change ("Stop swallowing exceptions that happen in |
| WorkflowRun workflowRun = Optional.ofNullable(p.scheduleBuild2(0)) | ||
| .orElseThrow(AssertionFailedError::new) | ||
| .waitForStart(); | ||
| SemaphoreStep.success("wait/1", workflowRun); |
There was a problem hiding this comment.
If you suggest in the fix to only use SemaphoreStep.success("wait/1", null); here instead of replacing it with echo 'test' in the pipeline then let me know.
The newer jenkins-test-harness provided with this change does not any longer swallow exceptions on test teardown. See jenkinsci/jenkins-test-harness#166 This reveals problems in the implementation and tests. Under Windows tests failed because some folders could not be removed due to open file handles. Fixes: 1) In `BaseFileContent` the `InputStream` was not closed correctly in every case. 2) The `SemaphoreStep` was not used properly and anyway not required in most cases. It has to be used correctly which requires 1-2 lines of code in every test to not leave the environment in a bad state. The platform-independent `echo` step is sufficient and requires no extra code. Using no step at all may also be an option.
|
@darxriggs please don't use force pushs because we lose history and it's harder to review. |
This is to stay up-to-date and use the latest features and fixes.