False positive mutant in Angular controller

[bkimminich]

I have this code in an Angular 1.x controller:

    $scope.showDetail = function (id) {
      $uibModal.open({
        templateUrl: 'views/ProductDetail.html',
        controller: 'ProductDetailsController',
        size: 'lg',
        resolve: {
          id: function () {
            return id
          }
        }
      })
    }

My original test simply did expect($uibModal.open).toHaveBeenCalled which lets this mutant survive: $scope.showDetail = function (id) {}.

Performing this mutation manually yields a failing test:

    Expected spy open to have been called with [ { templateUrl : 'views/ProductDetail.html', controller : 'ProductDetailsController', size : <jasmine.any(function String() {
        [native code]
    })>, resolve : { id : <jasmine.any(function Function() {
        [native code]
    })> } } ] but it was never called.

I even extended my test to be more specific about the expected modal dialog: expect($uibModal.open).toHaveBeenCalledWith({ templateUrl: 'views/ProductDetail.html', controller: 'ProductDetailsController', size: jasmine.any(String), resolve: { id: jasmine.any(Function) } })

The mutant $scope.showDetail = function (id) {} still survives. How is that possible?

[nicojs]

Hmm this indeed should not happen. Can you confirm that your test did ran for that mutant? Stryker performs analysis on code coverage. A bug there might result in this test not being ran for the mutant.

To see this, please run stryker with the clear-text reporter. You can also explicitly disable code coverage analysis by setting testSelector to null.

[bkimminich]

Running clear-text shows 15 surviving mutants, but the mentioned one is not among them. So I guess it is one of the 11 mutants untested.?

240 mutants tested.
11 mutants untested.
0 mutants timed out.
225 mutants killed.
Mutation score based on covered code: 93.75%
Mutation score based on all code: 89.64%

[bkimminich]

Using testSelector: null makes the tests go slower and seems to kill 2 extra mutants, but not this one.

[simondel]

Could you change your loglevel to debug (logLevel: 'debug')? That will show you the tests that ran for every mutant instead of just the killed ones.

[bkimminich]

It's among the untested ones:

[2016-09-15 15:04:27.114] [DEBUG] ClearTextReporter - Mutant untested!
[2016-09-15 15:04:27.114] [DEBUG] ClearTextReporter - C:\Data\Github\juice-shop\app\js\controllers\SearchResultController.js: line 13:38
[2016-09-15 15:04:27.114] [DEBUG] ClearTextReporter - Mutator: BlockStatement
[2016-09-15 15:04:27.114] [DEBUG] ClearTextReporter - -       $scope.showDetail = function (id) {
[2016-09-15 15:04:27.114] [DEBUG] ClearTextReporter - -         $uibModal.open({
[2016-09-15 15:04:27.114] [DEBUG] ClearTextReporter - -           templateUrl: 'views/ProductDetail.html',
[2016-09-15 15:04:27.114] [DEBUG] ClearTextReporter - -           controller: 'ProductDetailsController',
[2016-09-15 15:04:27.114] [DEBUG] ClearTextReporter - -           size: 'lg',
[2016-09-15 15:04:27.114] [DEBUG] ClearTextReporter - -           resolve: {
[2016-09-15 15:04:27.114] [DEBUG] ClearTextReporter - -             id: function () {
[2016-09-15 15:04:27.114] [DEBUG] ClearTextReporter - -               return id
[2016-09-15 15:04:27.114] [DEBUG] ClearTextReporter - -             }
[2016-09-15 15:04:27.114] [DEBUG] ClearTextReporter - -           }
[2016-09-15 15:04:27.114] [DEBUG] ClearTextReporter - -         })
[2016-09-15 15:04:27.114] [DEBUG] ClearTextReporter - -       }
[2016-09-15 15:04:27.114] [DEBUG] ClearTextReporter - +       $scope.showDetail = function (id) {
[2016-09-15 15:04:27.114] [DEBUG] ClearTextReporter - +   }
[2016-09-15 15:04:27.114] [DEBUG] ClearTextReporter -

If you want to locally reproduce this problem, just clone http://github.com/bkimminich/juice-shop and execute npm install and then npm run stryker.

[nicojs]

Thanks @bkimminich! That will be a huge help! I'm on a small vacation and will take a look at it soon as I get back, if @simondel doesn't fix it first that is.

[simondel]

When Stryker selects tests for a mutant, it looks at which tests cover the smallest statement surrounding the mutated code. We do it this way because a mutation may be smaller than exactly one statement. If a test covers the smallest statement, we decide that it should be ran when testing the mutant.

So, what happens in this case?
We have the code:

$scope.showDetail = function (id) {
  $uibModal.open({
    templateUrl: 'views/ProductDetail.html',
    controller: 'ProductDetailsController',
    size: 'lg',
    resolve: {
      id: function () {
        return id
      }
    }
  })
}

and you would expect that the smallest statement would be pretty much the entire piece of code above but... Stryker thinks that the smallest statement in this case is :

return id

None of the tests cover this return statement (as shown by the actual regular code coverage report) so Stryker marks the mutant as untested. Something goes wrong in the MutantRunResultMatcher's findSmallestCoveringStatement function or one of the functions it calls.

[simondel]

Testing in the MutantRunResultMatcher for this mutant can be done with this if-statement:

if(mutant.filename === 'C:\\dev\\juice-shop\\app\\js\\controllers\\SearchResultController.js' && mutant.location.start.line === 13){
               //test stuff here such as logging
            }

[nicojs]

Hmmm i was thinking that the smallest statement algorithm was the one to blame for this bug. Even if testSelector is set to null, we still do code coverage analysis on the entire test suite. Seeing as no test cover exactly the statement return id, this can indeed be explained by the bug.

We should add these examples as test cases to the test suite of the MutantRunResultMatcher. I would consider this bug fixed if we indeed kill this mutant, as well as report exactly the same output for test selector = null and the default one.

@bkimminich thanks again for the example.

[simondel]

So back in june we changed the way we mutate code and store the information about the mutation (represented by the mutant class). This forced us to change the MutantRunResultMatcher. The code that was changed here should* have been:

let mutantIsAfterStart = mutant.location.start.line > location.start.line ||
  (mutant.location.start.line === location.start.line && mutant.location.start.column >= location.start.column);
let mutantIsBeforeEnd = mutant.location.end.line < location.end.line ||
  (mutant.location.end.line === location.end.line && mutant.location.end.column <= location.end.column);

* I haven't thoroughly tested if this doesn't break anything else. It just fixes this bug and this code makes sense. It actually checks if the mutant starts after the start of the statement and if it ends before the end of the statement.

@bkimminich FYI Stryker now reports the following on your codebase:

247 mutants tested.
4 mutants untested.
0 mutants timed out.
230 mutants killed.
Mutation score based on covered code: 93.12%
Mutation score based on all code: 91.63%

The drop in the mutation score for covered code is caused by the fact that 2 more mutants survived. We'll have to take a look to see if those mutants should have survived or not.

[nicojs]

@simondel fix works like a charm! See PR #155. I added the unit test to prevent this regression in the future.

[nicojs]

@bkimminich we just released Stryker 0.4.4 with the fix. Can you confirm that this issue is now solved?

[bkimminich]

The mutant for the entire function now dies as expected. The surviving mutant within is correct, because my tests don't actually check the return value of the id() function inside resolve.