Provide a finer grain control on mutators to enable

[echebbi]

Motivation

As suggested by @LorenzoBettini in #77 (here)

Proposed Solution

Enhance the preferences page with a table displaying the full list of options (we should be able to support all the options offered by Pitest) which a checkbox to enable/disable each.

[JKutscha]

I would like to work on that topic, but would like to add this in the run config to be more flexible.
(Add a table with checkboxes of each mutator in separate tab)

[LorenzoBettini]

That's great! But please, keep in mind that you should also write a few UI tests for such new features.
Until #140 is merged to master, you should start from the corresponding branch. You can take inspiration from the tests in the project org.pitest.pitclipse.ui.tests and the package org.pitest.pitclipse.ui.tests.

[JKutscha]

I have looked into it, I just realized there are 150 mutators. Would you think to have full control over them would be nice or should we just present some of them?

[echebbi]

Hey there! Just so you know I've already worked on this feature in the 114-provide-finer-grain-control-over-mutators branch. It's a bit old (about 1 year old) so I don't know if it can be rebased on master easily but you should at least be able to reuse some code.

The branch adds a table to the Pit tab in the Run Configuration view that allows to select the mutators to run (the user can either selects pre-defined groups or selects a custom subset of mutators):

I think I added all the mutators that were described in Pitest's website at that time; they are all defined in the Mutators enum.

I think that I was almost done with the UI part but didn't start to actually use it to configure the Pitest run. So the performApply is likely not implemented in the Mutators tab to persist the preferences and the Pitest runner doesn't get the selected mutators either.

Notice that with my implementation the user can also select a pre-defined subset of mutators (DEFAULTS, STRONGER or ALL) which corresponds to a group defined by Pitest — looks like there is now an OLD group as well. Groups are defined in the MutatorsGroup enum but I don't think I linked them with the mutators they contain though.

[JKutscha]

Thanks for the quick reply.

I will take a look at your code and would like to use the mutators from pitest itself from the class "Mutator" in the package "org.pitest.mutationtest.engine.gregor.config". That would bring the benefit, if a new mutator gets added to pitest, the mutator would be selectable without any new implementation in pitclipse.

Any thoughts on that idea?

[JKutscha]

A different idea I had was that you could use the defined groups of the "Mutator" class to have different sections of the selection.

[echebbi]

I [...] would like to use the mutators from pitest itself from the class "Mutator" in the package "org.pitest.mutationtest.engine.gregor.config". That would bring the benefit, if a new mutator gets added to pitest, the mutator would be selectable without any new implementation in pitclipse.

Indeed. And Pitclipse wouldn't break if Pitest renamed or removed some mutators. However since Pitest provides neither a human-readable name nor a description of its mutators then we'd still have to implement them anyway (which shouldn't be a big deal, just saying that although it could make upgrading Pitest safer it doesn't remove all the work).

Moreoveor it looks like raw mutators and groups are stored in the same MUTATORS map. I think we could filter groups out (based on the number of items in the map's values or by specific keys) but we should ensure that's feasible before committing to that solution. Have you taken a look at this problem?

Do you know if this class is API? If it's likely to change then we may want to avoid using it.

A different idea I had was that you could use the defined groups of the "Mutator" class to have different sections of the selection.

It looks like there is about a dozen groups defined in this class and I don't know if it's really relevant to provide so many of them (I'm not a regular user of Pitest). If it does make sense to provide them all then we the UI should has a combo box rather than the list group I implemented.

Besides that it has about the same pros & cons than querying the mutators right from Pitest's Mutators class.

[JKutscha]

[...] I think we could filter groups out (based on the number of items in the map's values or by specific keys) but we should ensure that's feasible before committing to that solution. Have you taken a look at this problem?

Not yet, but I plan to look at this today or tomorrow.

[...] Do you know if this class is API? If it's likely to change then we may want to avoid using it.

I don't know if it is Api. How would I know if it is? Besides asking in the repo of pitest.

[JKutscha]

For now I will use your implementation and get it working.

[LorenzoBettini]

I think it is crucial to keep this completely maintainable and completely self-updating when updating to a new version of PIT. For this reason, IMHO, we cannot afford to hardcode anything and take what's available from PIT itself. Maybe @hcoles can confirm that there's some kind of API to retrieve Mutators descriptors? I would stick with IDs, which, if I remember correctly should be quite clear to understand and skip human-readable descriptions. After all, if anyone wants to have such a power on the configuration, they might have to be already aware of Mutators identifiers?

[JKutscha]

How about a third option: We use the mutators given by pit to build the table for selection and use the hard coded descriptions for all mutators which are currently implemented.

If new ones get added, the description could say: "No description yet, look at pitest.org for more info".

[echebbi]

LorenzoBettini: Maybe @hcoles can confirm that there's some kind of API to retrieve Mutators descriptors?

That would indeed be ideal.

LorenzoBettini: I would stick with IDs, which, if I remember correctly should be quite clear to understand and skip human-readable descriptions.

Some IDs are not intuitive at all: UOI, AOR, CRCR. And often not enough to figure out the whole effect of the mutator:

• MATH (ok, this mutator is likely related to mathematical computations, but what does it mutate exactly?),
• TRUE RETURNS (it doesn't mutate the return true statements as we might expect but instead replace all boolean return by true).

It may not be a big deal if the user is already used to Pitest, but...

LorenzoBettini: After all, if anyone wants to have such a power on the configuration, they might have to be already aware of Mutators identifiers?

I disagree. Pitclipse aims at easing mutation testing of Java projects within an Eclipse environment and as such I believe that we should expose as little internal details as possible. Users may start to use Pitclipse without prior knowledge of Pitest and I'd rather provide them all the info they need right in the IDE rather than asking them to look at some doc on the internet. Even Pitest users may be unsure about the meaning of an ID and may have to take a look at the doc, which feels inconvenient.

Providing a comprehensible table, with a readable name and a clear description, may require a bit more work from our side but IMHO it also greatly enhances the user experience.

JKutscha: If new ones get added, the description could say: "No description yet, look at pitest.org for more info".

Yes, that's what I expect if we decide to gather the mutators from Pitest; ideally we would provide the description at the same time we update Pitest though. The "no description" text would be a placeholder in case we fail to notice the addition of a new mutator.

JKutscha: We use the mutators given by pit to build the table for selection and use the hard coded descriptions for all mutators which are currently implemented.

@JKutscha I'm not sure I understand you. What I envision is that:

1. We query the list of all mutator IDs from Pitest
2. We display those mutators in a table, providing for each a human-readable name and a description through custom ColomnLabelProvider.
3. We persist the selected group/mutators in the RunConfiguration (i.e., their ID),
4. The Pitest runner retrieves the selected mutators from the RunConfiguration and gives their ID to Pitest.

Hence when upgrading Pitest we'd just have to make sure the label providers are up-to-date. Everything else woud be automatic. Does your "third option" differ from that?

LorenzoBettini: I think it is crucial to keep this completely maintainable and completely self-updating when updating to a new version of PIT. For this reason, IMHO, we cannot afford to hardcode anything and take what's available from PIT itself.

@LorenzoBettini I agree but as I said above I also think that providing a clear table would come in handy for users. With the solution I proposed Pitclipse would be automatically updated, we'd just have to update the label adapter when the mutators provided by Pitest change (assuming that there's no existing API to get a mutator description). IMO that's acceptable. What do you think about that?

[JKutscha]

We query the list of all mutator IDs from Pitest
We display those mutators in a table, providing for each a human-readable name and a description through custom ColomnLabelProvider.
We persist the selected group/mutators in the RunConfiguration (i.e., their ID),
The Pitest runner retrieves the selected mutators from the RunConfiguration and gives their ID to Pitest.

I was talking about this behavior, but wasn't able to describe it good enough.

[LorenzoBettini]

LorenzoBettini: I think it is crucial to keep this completely maintainable and completely self-updating when updating to a new version of PIT. For this reason, IMHO, we cannot afford to hardcode anything and take what's available from PIT itself.

@LorenzoBettini I agree but as I said above I also think that providing a clear table would come in handy for users. With the solution I proposed Pitclipse would be automatically updated, we'd just have to update the label adapter when the mutators provided by Pitest change (assuming that there's no existing API to get a mutator description). IMO that's acceptable. What do you think about that?

@echebbi as long as any of you commit to keeping it up-to-date I'm perfectly fine ;) O:)

[JKutscha]

I now have looked into it a little more. The problem I see currently there is no way to get the keys of the mutators, which can be given to the cli of pit to use the mutator.

Also it seems to there is no actual naming theme of the keys, which would allow to generate them from their names.
Example:
name: EMPTY_RETURN_VALUES
key: EMPTY_RETURNS
or
name: BOOLEAN_TRUE_RETURN
key: TRUE_RETURNS
or
name: INLINE_CONSTANT_MUTATOR
key: INLINE_CONSTS

[echebbi]

@echebbi as long as any of you commit to keeping it up-to-date I'm perfectly fine ;) O:)

@LorenzoBettini Ahah, fine, I'll take care of that.

I now have looked into it a little more. The problem I see currently there is no way to get the keys of the mutators, which can be given to the cli of pit to use the mutator.

Also it seems to there is no actual naming theme of the keys, which would allow to generate them from their names.

@JKutscha I didn't notice that Pitest's MUTATORS map is private. Looks like there's no easy way to access it or to browse its keys. So... yeah, I guess we have to hardcode the list of available mutators after all. It would be a shame to hardcode it on our side given that it's already defined in Pitest though.

Would you mind creating an issue in the Pitest repository to explain our situation and ask for a way to programmatically query all the IDs of mutators handled by Pitest? If they have no API yet we might want to propose to contribute by adding a new method that does the job in Mutator.java, something like getMutatorIds().

[LorenzoBettini]

@echebbi as long as any of you commit to keeping it up-to-date I'm perfectly fine ;) O:)

@LorenzoBettini Ahah, fine, I'll take care of that.

Great! :)

I now have looked into it a little more. The problem I see currently there is no way to get the keys of the mutators, which can be given to the cli of pit to use the mutator.
Also it seems to there is no actual naming theme of the keys, which would allow to generate them from their names.

@JKutscha I didn't notice that Pitest's MUTATORS map is private. Looks like there's no easy way to access it or to browse its keys. So... yeah, I guess we have to hardcode the list of available mutators after all. It would be a shame to hardcode it on our side given that it's already defined in Pitest though.

For the time being we could do a "dirty" thing: access the map via reflection?

Would you mind creating an issue in the Pitest repository to explain our situation and ask for a way to programmatically query all the IDs of mutators handled by Pitest? If they have no API yet we might want to propose to contribute by adding a new method that does the job in Mutator.java, something like getMutatorIds().

Of course, asking for an API is the best choice!

[JKutscha]

Would you mind creating an issue in the Pitest repository to explain our situation and ask for a way to programmatically query all the IDs of mutators handled by Pitest? If they have no API yet we might want to propose to contribute by adding a new method that does the job in Mutator.java, something like getMutatorIds().

@echebbi I would create the issue and a corresponding PR. This shouldn't be much effort to provide that.

Seeing all the open issues and PRs, my hope of getting it integrated is quite low. At least for the next few days or weeks.

[echebbi]

@echebbi I would create the issue and a corresponding PR. This shouldn't be much effort to provide that.

Thank you very much @JKutscha!

Seeing all the open issues and PRs, my hope of getting it integrated is quite low. At least for the next few days or weeks.

If it's just a matter of days then it's not a big deal. If the PR takes too much time to be integrated or if it's refused then we'll consider another solution. As @LorenzoBettini suggested, reflection could be a quick & dirty fix.

[JKutscha]

[...] As LorenzoBettini suggested, reflection could be a quick & dirty fix.

@echebbi I implemented it with reflection in the draft and it works. I marked it to be replaced, if the PR is merged and we can use it.

[LorenzoBettini]

[...] As LorenzoBettini suggested, reflection could be a quick & dirty fix.

@echebbi I implemented it with reflection in the draft and it works. I marked it to be replaced, if the PR is merged and we can use it.

Now that we use PIT 1.6.8, you can access them without reflection

[JKutscha]

Done, forgot to push that change.

[contivero]

Sorry if this isn't the right place, but it seems although this has been merged, it hasn't been released yet (at least it's not part of version 2.1.2 which is the one available on Eclipse). If so, how could I try it (is there any guide on how to build the latest state of this repo and use it in Eclipse)?

There is a bunch of mutators I'd like to enable without going for the "all mutators" option.

Thanks in advance!

[LorenzoBettini]

@contivero you're right: it hasn't been released yet, and I had always forgotten about that. @echebbi do you think it's time we make a new release with the new features? I can do that in the next few days.