pitest icon indicating copy to clipboard operation
pitest copied to clipboard

Mutate annotations =)

Open velo opened this issue 8 years ago • 9 comments

So, in a few framework we end up programming on annotations

Like:

interface GitHub {
  @RequestLine("GET /repos/{owner}/{repo}/contributors")
  List<Contributor> contributors(@Param("owner") String owner, @Param("repo") String repo);
}

https://github.com/Netflix/feign

So, I would like to have a Mutator that would go and delete the annotation with runtime retention.

I would be willing to write this, just need initial pointers of where to put this.

velo avatar Apr 11 '16 04:04 velo

It should be relatively straightforward to strip out runtime annotations. What makes this tricky is that pitest has an inbuilt assumption that a mutation occurs on a line of executable code - this is how the tests to be run are selected.

Some work would need to be done to break this assumption and allow a mutation to map to multiple lines.

hcoles avatar Apr 11 '16 20:04 hcoles

I think that will be really tricky and I'm not sure whether this will work at all. Annotations are Meta-Data - the code annotated might not have something to do with the code that extracts and analyzes those annotations.

I think about Springs @Transactional or @Component that might not have any effect in unit tests but might have an effect for integration tests.

The framework implemented by the company I'm working for uses JPA @Column annotations to analyze and configure validations for the GUI as well as hibernate might use them to create SQL but they won't have any effect to Unit Tests. Removing those Annotations will lead to lots of false positives and lots of tests using our entities/domain objects might be executed but none of them will kill any mutant. Same might be true for @JsonProperty and other mapping/orm frameworks.

Although I understand the use case described by @velo I'm not sure whether there would be a serious need or interest in limiting the annotations mutated using a whitelist or blacklist approach.

StefanPenndorf avatar Apr 19 '16 20:04 StefanPenndorf

It can be off by default...

velo avatar Apr 19 '16 20:04 velo

That's true and I think it really should but that's not the point.

Maybe I wasn't clear enough: Even if there are Annotations that can be mutated this might be usefull for a small number of real world projects only because they contain additional annotations that are not or cannot be covered by tests.

Maybe you can examine the project you have in mind. Are the Feign Annotations the only ones you use throughout the whole project? Maybe you can share a complete or partial list of all annotations used in your project. For how many of those can you write tests that detect their absence? Is it valuable to have X detected annotation removals but also have about the same amount of false positives in your reports? Maybe you're project is very different from the projects I'm facing ;-).

That's why I think there might be the need to define or configure the annotations that should be mutated and the ones that can be ignored (because you'll always have false positives when removing those).

StefanPenndorf avatar Apr 19 '16 21:04 StefanPenndorf

True... although even on JPA, you are programming in annotations... we just assume not testing is ok...

Another case where mutation would be really useful is on JSR-303 (bean validation)...

Annotation carrying logic can be more common them I think....

velo avatar Apr 20 '16 01:04 velo

Any news on this?

IMHO the javax.validation.constraints.* Bean Validation annotations would be a valid use case (i.e. it would be reasonable to test the proper functioning of the validation)

MahatmaFatalError avatar Sep 03 '21 16:09 MahatmaFatalError

I was just implementing some Spring security things and thought how useful it would be to have annotation mutators.

Here are some use cases for which I believe this to be useful: • Validation annotations - is validation tested properly? • Security annotations - is security tested properly?

For example:

@Service
@EnableMethodSecurity
class CustomerService(
    @Autowired customerRepository: CustomerRepository,
) {
    @PreAuthorize("hasRole('ADMIN')")
    fun findAll(): List<Customer> = customerRepository.findAll()
}

There should be tests like these:

@ContextConfiguration(classes = [CustomerService::class])
@MockBean(CustomerRepository::class)
@ExtendWith(SpringExtension::class)
class CustomerServiceTest(
    @Autowired val customerService: CustomerService,
) {
    @Test
    fun `cannot list customers if unauthenticated`() {
         assertThrows<AuthenticationCredentialsNotFoundException> { customerService.findAll() }
    }

    @Test
    @WithMockUser
    fun `cannot list customers if authenticated but not having the ADMIN role`() {
        assertThrows<AccessDeniedException> { customerService.findAll() }
    }

    @Test
    @WithMockUser(roles = ["ADMIN"])
    fun `given there are no customers, findAll returns an empty list`() {
        val result = customerService.findAll()
        assertThat(result).isEmpty()
    }
}

Having a mutator that removes the annotations could be really useful to detect the strength of unit tests in terms of testing those behaviors (in this case security) of an application that are implemented using annotations.

How to find out which tests to run? As far as I understand, at the present moment, pitest determines what tests to run by matching coverage with mutators via lines. And this doesn't work out of the box with annotations.

As a start, could we use: • The first line of a method for method annotations (like @PreAuthorize) • The first line of each method for class annotations (like @EnableMethodSecurity) for tracking the coverage?

christianhujer avatar Mar 18 '23 09:03 christianhujer

@christianhujer Unfortunately there are multiple issues which make supporting this sort of thing messy and hard. In addition to questions about how to make something that isn't a bytecode instruction look like one, we also have to understand when and how frameworks process the annotations. Spring has singletons, caches etc that store the results of processing. Removing an annotation from a class after that processing has occured will have no visible effect.

We're investigating adding some support for this sort of thing at arcmutate, but it's unlikely to make it into the pitest repo.

hcoles avatar Mar 31 '23 16:03 hcoles

For anyone interested we now have a preview release that adds support for mutating validation mutations for projects using spring or spring boot.

https://docs.arcmutate.com/docs/spring.html

We hope to expand the list of mutated annotations in a future release.

hcoles avatar Apr 28 '23 12:04 hcoles