BITMAG-1266-prevent-permanent-test-files - #96
Conversation
…s directory anymore.
…-permanent-rest-files # Conflicts: # bitrepository-reference-pillar/src/test/java/org/bitrepository/pillar/integration/perf/GetFileStressIT.java
ole-v-v
changed the title
ole-v-v
left a comment
ole-v-v
left a comment
There was a problem hiding this comment.
I suggest making the temporary directory a parameter to the test method rather than a static field. I have a couple of other minor suggestions.
The change is carried out nicely and elegantly.
|
|
||
| @Test | ||
| @Tag(PillarTestGroups.PILLAR_STRESS_TEST) | ||
| void noIdentfyGetFilePerformanceTest() throws Exception { |
There was a problem hiding this comment.
My comment above implied:
void noIdentifyGetFilePerformanceTest(@TempDir Path tempDir) throws Exception {
I have taken the opportunity to correct the typo in Identfy. You may correct in line 141 too if you like.
Depending on taste one may also narrow down the exception type thrown to IOException.
| public class GetFileStressIT extends PillarPerformanceIT { | ||
| public static final String FOLDER_NAME = "src/test/resources"; | ||
| @TempDir | ||
| static Path tempDir; |
There was a problem hiding this comment.
Rather than a field here would we want to reduce the scope by making it a parameter to the method that uses it? See my comment to the method header.
| httpServerConfiguration.getURL(fileName).toExternalForm(), | ||
| defaultFileId, null, getPillarID(), getPillarID(), | ||
| settingsForTestClient.getReceiverDestinationID(), pillarDestination); | ||
| getRequest.setFileAddress("file:" + tempDir.toAbsolutePath().toString()+"/"+fileName); |
There was a problem hiding this comment.
Is this necessary? The test is also green without it.
If we want it (for one reason or another), conventions say to put a space before and after each +, which I also find nicer to read.
Matter of taste, we don’t need the toString() call since toString() is implicitly called in the string concatenation.
| getLimiter.addJob(correlationID); | ||
| String fileName = nonDefaultFileId + "-" + i; | ||
|
|
||
| // Ensure the source file exists in the temp directory so the server can serve it |
There was a problem hiding this comment.
Is this necessary? The test is also green without it.
2 participants
Removed that the test files got stored mainly by adding @tempdir