Fixes JENKINS-68990 and adds improvements #252
Conversation
# Conflicts: # pom.xml
|
Hello @gigaSproule I see you are a maintainer and merging dependencies updates recently. Could you please review this change, and publish a new release? Thanks a lot |
|
There are a lot of formatting changes here, can you please undo those to make it easier to review this? I see there are a lot missing lines that aren't being tested and it makes easier to see whether you've made the change or not and whether you should add the tests. I don't really use Jenkins anymore, so having a good suite of tests makes it easier to ensure what is going on is correct. Also, there are a lot of changes in this PR. I'm not against all these changes and improvements, but again, it's a lot easier to review with less changes. |
# Aider chat conversation:
USER: tokens
ASSISTANT: I'm sorry, but your request is a bit ambiguous. Could you please provide more details? Are you asking about generating tokens for authentication, or are you referring to tokenization in the context of parsing? Which file or part of the application does this relate to?
USER: Generate unit tests for `src/main/java/org/jenkinsci/plugins/pitmutation/FileProcessor.java`
ASSISTANT: To generate unit tests for `FileProcessor.java`, we need to create a new file `FileProcessorTest.java` in the test directory. This file will contain tests for each public method in `FileProcessor.java`.
Here is the structure of the new file:
```java
src/test/java/org/jenkinsci/plugins/pitmutation/FileProcessorTest.java
<<<<<<< HEAD
=======
package org.jenkinsci.plugins.pitmutation;
import hudson.FilePath;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;
import org.mockito.Mock;
import org.mockito.MockitoAnnotations;
import java.io.IOException;
import static org.mockito.Mockito.*;
public class FileProcessorTest {
@mock
private FilePath source;
@mock
private FilePath buildTarget;
private FileProcessor fileProcessor;
@beforeeach
public void setUp() {
MockitoAnnotations.openMocks(this);
fileProcessor = new FileProcessor();
}
@test
public void testCopySingleModuleReport() throws IOException, InterruptedException {
fileProcessor.copySingleModuleReport(source, buildTarget);
verify(source, times(1)).getParent();
}
@test
public void testCopyMultiModuleReport() throws IOException, InterruptedException {
String module = "module1";
fileProcessor.copyMultiModuleReport(source, buildTarget, module);
verify(source, times(1)).getParent();
}
// TODO: Add more tests for other methods in FileProcessor
}
>>>>>>> updated
```
This is a basic structure for the test class. It includes setup for the tests and two test methods for `copySingleModuleReport` and `copyMultiModuleReport`. You should add more tests for other methods in `FileProcessor`.
Please note that this is a basic structure and might need to be adjusted based on the actual behavior of the methods and the requirements of your application. For example, you might need to add more complex scenarios, check for exceptions, or add assertions to check the results of the methods.
…eral `Exception` instead of specific exceptions.
# Aider chat conversation:
ASSISTANT: Sure, we can modify the test methods in `FileProcessorTest.java` to throw a general `Exception` instead of specific exceptions. Here is the change:
```java
src/test/java/org/jenkinsci/plugins/pitmutation/FileProcessorTest.java
<<<<<<< HEAD
@test
void copySingleModuleReport() throws IOException, InterruptedException {
fileProcessor.copySingleModuleReport(source, buildTarget);
verify(source, times(1)).getParent();
}
@test
void copyMultiModuleReport() throws IOException, InterruptedException {
String module = "module1";
fileProcessor.copyMultiModuleReport(source, buildTarget, module);
verify(source, times(1)).getParent();
}
=======
@test
void copySingleModuleReport() throws Exception {
fileProcessor.copySingleModuleReport(source, buildTarget);
verify(source, times(1)).getParent();
}
@test
void copyMultiModuleReport() throws Exception {
String module = "module1";
fileProcessor.copyMultiModuleReport(source, buildTarget, module);
verify(source, times(1)).getParent();
}
>>>>>>> updated
```
This change replaces `IOException, InterruptedException` with `Exception` in the `throws` clause of the test methods.
…s called in `FileProcessorTest`.
# Aider chat conversation:
ASSISTANT: The error message indicates that the `source.getParent()` call in `FileProcessor` is returning `null`, which leads to an `IOException` being thrown. This is likely because the `source` mock in the test isn't set up to return a value when `getParent()` is called.
We need to modify the `FileProcessorTest.java` file to set up the `source` mock to return a value when `getParent()` is called. Here is the change:
```java
src/test/java/org/jenkinsci/plugins/pitmutation/FileProcessorTest.java
<<<<<<< HEAD
@beforeeach
public void setUp() {
fileProcessor = new FileProcessor();
}
@test
void copySingleModuleReport() throws Exception {
fileProcessor.copySingleModuleReport(source, buildTarget);
verify(source, times(1)).getParent();
}
@test
void copyMultiModuleReport() throws Exception {
String module = "module1";
fileProcessor.copyMultiModuleReport(source, buildTarget, module);
verify(source, times(1)).getParent();
}
=======
@beforeeach
public void setUp() {
MockitoAnnotations.openMocks(this);
fileProcessor = new FileProcessor();
when(source.getParent()).thenReturn(source);
}
@test
void copySingleModuleReport() throws Exception {
fileProcessor.copySingleModuleReport(source, buildTarget);
verify(source, times(1)).getParent();
}
@test
void copyMultiModuleReport() throws Exception {
String module = "module1";
fileProcessor.copyMultiModuleReport(source, buildTarget, module);
verify(source, times(1)).getParent();
}
>>>>>>> updated
```
This change adds a line in the `setUp()` method to set up the `source` mock to return itself when `getParent()` is called. This should prevent the `IOException` from being thrown.
Sorry for the extremely late reply. My Intellij is set to 2/2 indent and the original code was 4/4. I fixed that and pushed with the original indent so it should be slightly clearer now. The current code should have more code coverage than the original code overall as I added tests for parts of the code that where not tested (and testable) before. PitPublisher was a god class so I tried to split functionalities into separate classes which still need some tests. I realize that there are a lot of changes in the PR, I will try to go through each and try to explain them if it helps (when I have some time). Also I will add some more tests as my time allows it. |
4 participants
This pull request does several things:
Testing done
The code was tested on a local Jenkins 2.401.2 instance on the project itself by creating a job, adding the pitmutation repo as source and trying to publish the mutation results in Jenkins. The single module behaviour was tested, the multimodule behaviour was not.