Function chain

[wolf-off]

pipe/chain functions.
It is popular and very widespred to use pipe functions or some chains.

mycollection.filter(e=>e.active).map(e=>e.value).sort();

And very usefull to remove one call from chain to verify tests

mycollection.map(e=>e.value).sort();

mycollection.filter(e=>e.active).sort();

mycollection.filter(e=>e.active).map(e=>e.value);

May be it's not good for some projects and can generate some lifeless mutants,
but my experience show, that this is one of usefull mutator,
when you use array pipes or work with RxJs

I'm using this implementation
https://github.com/wolf-off/stryker-rxjs/blob/master/src/mutator/FunctionChainMutator.ts

[brodycj]

Welcome @wolf-off to the community, I was thinking something similar before. I definitely like your idea and the way you were able to make a separate working mutator as a demo.

As an outside contributor, I wonder if we could somehow make it easier or smoother to make individual mutator plugins like yours and the ideas I had proposed in PR #1891 work together. Just food for thought.

Another thing is that we have separate mutator implementations for JavaScript and TypeScript, I raised discussion in #1893 to hopefully bring them together somehow.

[bartekleon]

If you remove 1 function from chain, you will get n - 1 mutants, where n is the original chain length. If you do it for all of them, you will get (n - 1)!. I dunno if it is the best mutator, it surely would work in some cases, but in general it won't be commonly used. Instead of hard-coding it in Stryker I would rather opt for making it an optional plugin.

Also consider other cases. Like:
test.map((e)=>e.value);

test.sort((e) => { a lot of stuff here })

test.myOwnFunction().myAnotherFunction();

[wolf-off]

1. 2^n mutants are in your vision. n! is too much dublicates
2. I offer to remove 1 function once. n-chain generates n mutants
   It's enough to check is there test for every call.
3. Problem that in low reactive/pipe/chain code there are no much function that return the same as 'this'. Deleting of any function in midle just destroy execution for it:

myCar.getEngine().getCompany().isCompany('Ford')

But line like this are not popular too and we will not create too much mutants (3 here)
4. On other side:

mycollection.filter(e=>e.active).map(e=>e.parent).sort();

myString.replace('+', '-').trim().substring(0,10).toUpperCase()

This chains currently are not mutated at all, just can be deleted now.
But there are base javascript functions used.
And there is not way to check Is some 'trim' or 'map' useful or rudiment

[bartekleon]

Yea. My mistake, I used equation for all possible combinations.
it would be 2^n-1. (+original)

This mutator still will behave differently / strange in some cases.
Like you said, ex. Any function with this

We could make list of "mutable chains", like [map, filter, sort, for each, etc.], Or check via some prototype or sth...
Ex "is map in object"
typeof Object.prototype[functionFromChain] === "function"

But it's still is tricky, I dunno if it is the best idea. You might want to try and make a plugin but I really don't know if it's enough consistent for core mutator

[wolf-off]

Previously there was request, but have not implemented
#418

[nicojs]

@wolf-off thanks for opening this issue!

I've discussed it with @simondel and we agree that we need these kinds of mutations. In fact, they are already implemented in some way in our Stryker.NET and Stryker4s sister projects.

We should align with them on the name and call it the Method expressions mutator

Instead of hard-coding it in Stryker I would rather opt for making it an optional plugin.

Let's make the first implementation with a hardcoded list and see what the reaction is. We can always make it configurable in the future.

I suggest focussing on strings and arrays first. There is some overlap between them and rxjs function names, so that's a bonus.

	Original	Mutated
✅	endsWith()	startsWith()
✅	startsWith()	endsWith()
✅	trim()
✅	trimEnd()	trimStart()
✅	trimStart()	trimEnd()
✅	substr()
✅	substring()
✅	toUpperCase()	toLowerCase()
✅	toLowerCase()	toUpperCase()
✅	toLocalLowerCase()	toLocalUpperCase()
✅	toLocalUpperCase()	toLocalLowerCase()
✅	sort()
✅	some()	every()
✅	every()	some()
✅	reverse()
✅	filter()
✅	slice()
✅	charAt()

There are a couple more we can think of, however, they will give a lot of false positives in the form of equivalent mutants.

	Original	Mutated
❌	pop()	shift()
❌	reduce()	reduceRight()

Another thing is that we have separate mutator implementations for JavaScript and TypeScript, I raised discussion in #1893 to hopefully bring them together somehow.

We will have to implement it twice for now. They do share the same tests, so its not that big of a deal.

[wolf-off]

I wanna add info about some function like toUpperCase and trimStart:

In my vision is better to change it to none.
When we change toUpperCase function to toLowerCase function, mutant will alife only if all tests use and check variable from intersection - {toLowerCase}∩{toUpperCase}
It is not small set - some digits, special chars
But if we change toUpperCase function to none it allows alife more wide set - whole {toUpperCase} set

Example:

myFunc(val) { return val.toUpperCase(); }
 expect(myFunc('123')).toEqual('123');
 expect(myFunc('QWE')).toEqual('QWE');

Mutant with toLowerCase replacement will not alive, but in reallity there are no checking for toUpperCase function
Mutant with none replacement will alive and show that there are week test.

[bartekleon]

Agree to that ^ creating fake mutants is not good. That's also why I am a bit against this type of mutation - we have to think of lots of cases not to do silly mistake like that one ^

Creating more equivalent mutants and fake mutant for another complexity of Stryker isn't the best scenario IMHO.

For each string method. What will happen if I use empty string or string only with special characters?

StartsWith and endsWith - "anna".
Trim start/end - " ".
"(Any character)".charAt(0)
substring, substr - just get whole string

Array:
[].sort/reverse/filter
["1","2","1"].reverse()
[1,1,1].filter()
["1"].some()

[MrFix93]

@kmdrGroch I get the point, and I agree that creating fake mutants is probably not a good idea. However it might create for a lot of cases useful mutants. Can't we do both? Create mutants that remove the method from the chain aswel as changing it to their opposite method?

I have some time for implementing these kind of functionality, so if we can agree upon a solution, I might be able to implement this. No pressure though.

[bartekleon]

Yea, you could try. And I was thinking, that you can check, if original output is equal to mutated one, e.x [].filter() === [], and if that's a case, don't mutate, or mark it somehow that it doesn't produce a valid mutant... @nicojs your ideas about it? @wolf-off do you think it would reduce amount of equivalent mutants enough? @MrFix93 any other ideas? Do you agree?

[nicojs]

In my vision is better to change it to none.
When we change toUpperCase function to toLowerCase function, mutant will alife only if all tests use and check variable from intersection - {toLowerCase}∩{toUpperCase}

I undestand that '123'.toUpperCase() === '123'.toLowerCase(). This is exactly the reason why '123' is not a great test case for a function that uses toUpperCase or toLowerCase. The result would be a survived mutant, which is exactly what you want. In order to kill the mutant, you would need a unit test to verify the inner workings of your unit with an actual letter.

Same story for

[].sort/reverse/filter
["1","2","1"].reverse()
[1,1,1].filter()
["1"].some()

@kmdrGroch I don't know exactly what "fake" mutants are, but if you're referring to "equivalent" mutants, I think we're safe. Removing a filter() for example, shouldn't lead to an equivalent mutant in practice. I agree that in your examples they will be equivalent mutants, but when would you use filter on a hard coded list AND have the resulting list be the same is the input?

I do agree that it's not productive to mutate pop -> shift or reduce -> reduceRight. Because there is a higher likelihood for those to generate equivalent mutants. See the following examples:

// An easy way to clear an array. 
while(arr.pop()) {} 
// Equivalent to:
while(arr.shift()){ }

arr.reduce((a, b) => a < b? a: b));
// Equivalent to:
arr.reduceRight((a, b) => a < b? a: b));

[bartekleon]

@nicojs you are right, it is totally my misunderstanding. Thanks for clarifing it. So basically we should avoid changing operations which work left - right like pop and shift, reduce, reduce right, trim left, trim right, trim. Instead, we should just remove it instead of changing i guess. Or make some double check - remove it AND replace.

[MrFix93]

I'm preparing a PR for this feature.

[simondel]

@MrFix93 Hi 👋 Do you have an update on this?

[MrFix93]

Offcourse. The javascript mutator is implemented and tested. However, I'm struggling with the integration tests. These test do not pass as the mutation score is different (offcourse). Any thoughts on how to handle this?

[simondel]

Awesome <3 You could validate if the outcome of the run is what you'd expect. If that's the case, there's no harm in updating the mutation score to the new value.

[MrFix93]

Awesome. Will do! I'm planning to create separate pull requests for the js and ts mutators, that's not a problem right?

[simondel]

That's fine by me! For adding tests, you could use the mutator-specification package. That way you will end up having the same test cases for javascript and typescript code :)

[stale]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

[nicojs]

Thanks stale, but we still want this feature.

[stale]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

[nicojs]

Thanks stale, but we still want this feature. Trust me, it will be awesome!

[LayZeeDK]

Yearly bump? 😅

[nicojs]

It actually isn't even a lot of work. Let's make it a stretch goal for v6 😍

[JoshuaKGoldberg]

Looks like a perfect issue for me to tackle 😁