Refactoring and tests for other utilities #73
Conversation
Pull Request Test Coverage Report for Build 7782239506Warning: This coverage report may be inaccurate.We've detected an issue with your CI configuration that might affect the accuracy of this pull request's coverage report.
💛 - Coveralls |
There was a problem hiding this comment.
Thanks @DurieuxPol!!
I like the changes.
I propose some renames and little API changes, what do you think about them?
| MTMutantOperatorAnalysisTest >> testGetAllOperators [ | ||
|
|
||
| | actual expected | | ||
| actual := (operatorAnalysis operatorsProducingOverXMutants: 0) keys |
There was a problem hiding this comment.
the name of this method is strange :)
operatorsProducingOverXMutants:
Does it mean they produce more than 0 mutants?
Alsy, why is it answering a dictionary? I see all your tests do keys asSet, keys asSet, keys asSet. This means that probably we want a method returning the set of keys directly and hide the fact that internally it's a dictionary right? :D
There was a problem hiding this comment.
Actually, Over 0 means > 0 or >= 0? Maybe a little rename could clarify "At least" vs "More than"
There was a problem hiding this comment.
I think it can be useful to return a dictionary, to actually see how many mutants are produced by each operator.
But yes I think I could add a bit more methods, like the whole dictionary, the set of mutants producing at least a certain number of mutants, the 10 most producing operators, things like that.
There was a problem hiding this comment.
Actually there is already a method returning the whole dictionary, forgot about it ahah
|
|
||
| | actual expected | | ||
| actual := (operatorAnalysis operatorsProducingUnderXMutants: 2) keys | ||
| asSet. |
There was a problem hiding this comment.
Here it's the same but the other way around.
- the
keys asSetlooks like the same smell (but duplicated code?) - under is < x or <= x ? :D
| ^ self | ||
| forPackages: aCollectionOfPackages | ||
| andTestPackages: | ||
| (aCollectionOfPackages collect: [ :package | package , '-Tests' ]) |
There was a problem hiding this comment.
I like this. This forces a convention!!!
|
|
||
| | analysis dic classesToMutate | | ||
| classesToMutate := aName asPackage definedClasses. | ||
| MTMutantOperatorAnalysis >> getMutantOperatorsDictionary [ |
There was a problem hiding this comment.
remember that in Pharo code we don't use the get/set prefixes in methods as a convention.
There was a problem hiding this comment.
Indeed, I will change that.
| @@ -76,23 +68,19 @@ MTMutantOperatorAnalysis >> getMutantOperatorsDictionaryFor: aName [ | |||
| ] | |||
|
|
|||
| { #category : 'computing' } | |||
| MTMutantOperatorAnalysis >> operatorsProducingOver: aNumber mutantsFor: aCollectionOfPackages [ | |||
| MTMutantOperatorAnalysis >> operatorsProducingOverXMutants: aNumber [ | |||
There was a problem hiding this comment.
Removing this extra parameter all over the code is indeed an improvement :)
Refactored
MTMutantOperatorAnalysisandMTMissingMutantOperatorAnalaysis(renamed asMTNonMutatedMethodsAnalysis) to support better APIs.Also added tests for them.