-
Notifications
You must be signed in to change notification settings - Fork 28
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat: change Pit output format to XML, close #616 #661
feat: change Pit output format to XML, close #616 #661
Conversation
Hi @andrewbwogi Thank you very much for the pull request! It looks great, I'm gonna review the code and keep you updated! :-) The failing is due to the checkstyle. You should use braces for if statement. |
The additional information seems okay for me. The method descriptor should stay as it is, since it a standard format to describe java methods signatures. |
|
||
public PitMutantScoreSelector() { | ||
this.testThatKilledMutants = new HashMap<>(); | ||
parser = new PitXMLResultParser(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Here, the parser is hardcoded. Could we use a command line option or a property to allow the user to use the format he wants, XML or CSV?
import java.io.IOException; | ||
import java.util.List; | ||
|
||
abstract public class AbstractParser { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We could use a generic here:
abstract public class AbstractParser<T extends AbstractPitResult>
return new File(directoryReportPit.getPath() + PATH_TO_MUTATIONS_RESULT); | ||
} | ||
|
||
public List<AbstractPitResult> parseAndDelete(String pathToDirectoryResults) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
And reuse the generic here:
public List<T> parseAndDelete(String pathToDirectoryResults) {
return results; | ||
} | ||
|
||
abstract public List<AbstractPitResult> parse(File fileResults); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same here.
} | ||
|
||
public PitMutantScoreSelector(String pathToOriginalResultOfPit) { | ||
this(); | ||
initOriginalPitResult(PitResultParser.parse(new File(pathToOriginalResultOfPit))); | ||
initOriginalPitResult(parser.parse(new File(pathToOriginalResultOfPit))); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We may have a problem here. In fact, the pathToOriginalResultOfPit
come from the command line.
I suggest that: we update the command line options (m|--path-pit-result)
in order to:
say that we support also the XML format and according to the given format, we use the correct parser.
Today, one may want to you the CSV format for the initial mutation score but want to use the XML as current output.
@oscarlvp could you check if you need something more? |
Any news on the requested changes? Did you encounter any trouble to implement them? |
I would like to make some manual tests but I get an error when running the DSpot jar:
I compile and package the project as instructed in the README except I skip tests, but that has not been a problem before. I will try to find whats wrong. Besides testing, your requested changes have been implemented. |
Hi @andrewbwogi looks great! Gonna check them further. For the manual testing, what are you doing?
Do you try to run the maven plugin with a |
I'm good with the changes, tell me when it is ready to be merged. |
I found the project=./
src=src/main/java/
testSrc=src/test/java
javaVersion=8
outputDirectory=dspot-out/ With the basic command: java -jar ./dspot-1.2.2-SNAPSHOT-jar-with-dependencies.jar -p ./dhell-dspot.properties \
--verbose Running a DSpot jar from an older commit (caba4a7) with this properties file on dhell works. Is this a known issue? |
Hi @andrewbwogi I do not know this issue. On my side, I cannot reproduce the stuck (DSpot ends well its execution). According to your logs, PIT is stuck. Could you try with: the Jacoco selector? i.e. Could you also try with the mutation engine Since I'm not able to reproduce the issue, it is hard to find the problem. |
With In test-projects, DSpot also freezes with the properties file: #relative path to the project root from dspot project
project=./
#relative path to the source project from the project properties
src=src/main/java/
#relative path to the test source project from the project properties
testSrc=src/test/java
#java version used
javaVersion=8
#filter used to amplify specific test cases
filter=example.*
#path to the output folder
outputDirectory=target/trash/
# Constant amount of additional time to allow a test to run for
# before considering it to be stuck in an infinite loop
timeoutConstInMillis=10000
#Argument string to use when PIT launches child processes. This is most commonly used
# to increase the amount of memory available to the process,
# but may be used to pass any valid JVM argument.
# Use commas to separate multiple arguments, and put them within brackets
jvmArgs=['-Xmx2048m','-Xms1024m']
#Mutators to apply when using Descartes Mode
#descartesMutators=['void','null','true','false','empty','0','(byte)0','(byte)1','(short)1','(short)2','0L','1L','0.0','1.0','0.0f','1.0f','\\40','\\'A\\'','\\"\\"','\\"A\\"']
descartesMutators=['\\'A\\''] With command: java -jar dspot-1.2.2-SNAPSHOT-jar-with-dependencies.jar -p ./test-projects2.properties When I run with java -jar dspot-1.2.2-SNAPSHOT-jar-with-dependencies.jar -p ./test-projects2.properties --test-criterion JacocoCoverageSelector --verbose Execution trace: out2.txt
I will try to find at what commit DSpot starts to freeze. |
Hi @andrewbwogi Where did you find the properties file? There are at least two errors in it: jvmArgs=['-Xmx2048m','-Xms1024m'] Since 38fe714, the jvmArgs must not wrapped by brackets, nor in single quote. They must only be separated by commas. You should have: jvmArgs=-Xmx2048m,-Xms1024m descartesMutators=['\\'A\\''] Same here, you should have: descartesMutators='A' Here, the single quotes express the fact that the mutator is working on char. Could you please:
Thank you very much @andrewbwogi ! :-) |
The properties file was a copy of a test-projects properties file from an old DSpot commit, I missed the updated syntax. However, even in the current DSpot commit, the use of descartesMutators is specified in the old way with brackets in that file. With the updated properties file, DSpot terminates with |
Suddenly, everything works. I've done manual tests now, the changes can be merged. |
Okay this is a great! We merged your pull request before the new year ;) |
The suggested tasks in #616 have been implemented. It remains to be decided what additional information in the PitXMLResult should be used in the final DSpot report. If the method signature of the mutated classes will be used, it could be presented in a more intuitive form. Currently it would be in Java bytecode format. @danglotb, what additional information do you think should be used, and is the current format of method signatures ok?