proposal and preview for mutagen v0.2

[samuelpilz]

As discussed in #142, this is a proposal for a modular architecture, which is the base for mutagen v0.2. I reviewed all parts of the code and tried to move code that works fine to the new system.

This is a minimal implementation to demonstrate the effectiveness of the architecture. The steps towards releasing mutagen v0.2 are:

• implement more mutators and document their functionality
• generate meaningful reports

Are there any other major tasks for this PR?

[samuelpilz]

I have done quite a few changes to the architecture since the first commit. I am quite happy with the current state.

• I added a crate mutagen-core that contains most code. This crate is imported from mutagen-transform and mutagen. This makes re-use of code a lot easier and still meets the requirement of a standalone crate for the procedural macro.
• The code to transform expressions is defined in the same file with their mutators.
• The hack for exhaustive testing has beed replaced by the module mutagen-selftest, which provides the same functionality in a cleaner way.
• I added an example-crate with well-known functions that help demonstrate the use of #[mutate] and the functionality of the runner. The runner can be called with cargo run --bin cargo-mutagen --package cargo-mutagen from the examples/simple directory

[llogiq]

Why are we losing this information? I feel it is a crucial piece to understand the design decisions for mutagen, which haven't really changed with your redesign.

[samuelpilz]

This information is indeed important. I thought I kept this information somewhere. I will add this paragraph back into the documentation and the readme.

[samuelpilz]

In the newest commits this section is back in the readme. The documents about the architecture and design decisions also contain paragraphs on this topic.

[llogiq]

Perhaps we should advise users to use sccache?

[samuelpilz]

I have no experience with that. I wanted to say that if a library uses mutagen, users of that library should not have to compile mutagen since it is only used for internal tests.

For testing purposes, the compile-time is fine. I guess I have to rewrite this section to avoid confusion about this topic.

[samuelpilz]

Updated in the newest commits. I think the new version is the best reason for using mutagen as a dev-dependency: "ensures nothing will land in released code"

[samuelpilz]

In the newest commit, I implemented another mutator that mutates comparison operators or PartialOrd into each other. I have some of other mutators planned, but I want to look at the current structure a bit and try to find rough edges in the design before implementing more mutators.

At this stage, it is still possible to change the architecture of everything. Do you have any requests / suggestions?

[llogiq]

Why do we panic here?

[samuelpilz]

As far as I understand:

• If the optimistic assumption fails, the mutant should not be considered survived.
• To be able to kill the mutant, the test suite has to fail.
• To fail the test suite, the code must have some different behavior when activating the mutant.
• I believe panicking is the only/easiest way to cause behavior that is different from the non-mutated case. However, I am open to other options.

In the future, I would like to read the output of the test-binary and detect if a certain panic-message has been printed. Based on that, the mutant could be considered "failed" instead of "killed" or "survived".

[llogiq]

Ok, that makes sense. I had envisioned a backchannel (an mmap, socket or simple file) to inform the test runner of the ineffective mutation on the no-mutation-run, so the runner could mark the mutation as ineffective. However, there is actually a tri-state at play here, because such a mutation could be within a generic function that monomorphizes to any number of ineffective mutations. Thus, we should be able to both mark an optimistic mutation as (potentially) ineffective and as effective (at least we should do so if it was marked as ineffective before and succeeds nonetheless).

[llogiq]

I should add that in-band messaging is inherently problematic and I would oppose misusing panics for that purpose. What if a user has a custom panic message that is inadvertently parsed as a killed mutant?

[samuelpilz]

I see that using panics might not be the cleanest solution. I will work on some backchannel for coverage-analysis. The information about optimistic assumptions can be included there.

However, I think that not necessarily part of a first working release, which I would like to finish in early September.

I did not think about optimistic mutantions in generic functions. I will investigate some issues here.

[llogiq]

I'm fine with that. Currently we just write a file on the nonmutated run, but there are likely faster and leaner solutions.

[llogiq]

Sorry for letting you wait for so long. I was on vacation. This looks pretty good so far, I only commented on a few questions I had.

I think this should be ready to merge once we reach feature parity with the previous version. Or would you like this to be merged sooner?

[samuelpilz]

Your suggestion is very promising. However, the that approach duplicates the input expressions, which is not allowed when the expressions move something out of a variable. I've not been able to use this idea to get a working transformation in all cases.

[samuelpilz]

I am quite happy with the current layout of the report and consider this as done for now. For the time being, I am also happy with the mutators implemented so far. I implemented mutators for numeric operations +, -, * and / as well as the bit-operations &, |, ^.

After that, I would like to publish the current state as mutagen-0.2. I added you to the authors in the Cargo.tomls, which was not the case during my mutagen-preview proposal. Is this ok? If necessary, I would write a post about the release. I will also improve my repository mutagen-applied that tries to show how mutagen could be applied to existing libraries.

[llogiq]

I'm currently very short on time, but will review soon. I'd also like to see some docs regarding open tasks for the future.

[samuelpilz]

I wrote a short list of in beyond mutagen-0.2. These tasks could give a roadmap for the mid-term future of mutagen.

[llogiq]

👍

[samuelpilz]

During my work for mutagen-applied, I found some more issues when applying mutagen to the crate bytes:

There is a line std::process::abort(); and its output type of ! is required.

The previos version rewrote the statement into if [...] {std::process::abort();}, which has type ().

I introduced a fix by generating the following code: if [...] {std::process::abort();} else {::[...]::stmt_call_to_none()}, which uses similar tricks to other optimistic mutators.

This also fixes the issues discussed above for f(return 1)-like cases. Further, I labelled the mutator for deleting such statements as optimistic.

[samuelpilz]

I just realized a major issue. Currently, we suggest adding #[cfg_attr(test, mutate)] to the code under test. Then, we run all test suites found by cargo test. However, the test flag is enabled for the unit tests directly in the lib only. It is not enabled for other tests (e.g. files from tests folder).

As I understand it: It is impossible for the macro mutate to be applied in integration tests while having mutagen in the dev-dependencies section

I see several ways to resolve this. I am also happy to discuss other approaches.

• only consider unit-tests as executed by cargo test --lib for mutation coverage and stick with the current suggestions.
• suggest that mutagen is feature gated. This implies a few things:
  • the crate has to introduce a new feature (e.g. mutationtest, suggested name can be discussed but cannot be named mutagen)
  • the dependency mutagen must be a real dependency, not just a dev-dependency and should be optional
  • the feature has to imply the dependency mutagen
  • the mutate attribute has to be guarded with #[cfg_attr(feature="mutationtest",mutate)]
  • the runner must enable the feature for the tests, hence it must know the name of the feature either by convention or configuration
  • It is no longer guaranteed by cargo that no mutated code is shipped with production

short example for a possible suggested Cargo.toml configuration

[features]
mutationtest = ["mutagen"]
[dependencies]
mutagen = {optional = true}

I am not sure where to go with this and which direction is the least dangerous or most useful.

[llogiq]

I would take the current approach and lobby the rust devs to set the test flag also when running integration tests.

In other news, David Tolnay has documented a way to hack together specialization on stable. I'm currently trying to introduce this to overflower, and it would fit well with mutagen, too.

[samuelpilz]

I am fine with the current approach. I am currently refactoring the runner to respect coverage measures of each testsuite individually.

However, I believe that the choice to disable the test flag and dev-dependencies inside the integration tests is based on the fact that they should test the crate "as any outside crate would". If possible, I will not do the lobbing necessary, but am happy with you or anyone else doing it.

I will try David Tolnay's method. However, we still require proc_macro_span feature for getting the filenames and locations for mutations. The tracking issue rust-lang/rust#54725 has made little progress. If we need to be on nightly anyway, I would like to stick to the "official but unstable" approach instead. If it is possible to provide the desired functionality without any unstable features, I will invest the effort to cut them all.

[samuelpilz]

I uncovered another issue. (see Post in users.rust-lang). This means that type inference rules can cause compilation errors in code that is not transformed by the mutagen. How should we deal with this?

[samuelpilz]

I did some improvements on the printed report and the running mechanism. My tests on mutagen-applied have been successful. Integrating mutagen into an existing project is not completely automatic and requires minimal amount of manual intervention in some cases. However, this effort is far smaller than the analysis of the mutationtest report, which is obviously a manual process.

I would like to merge and publish my proposed version of mutagen as v0.2. Is this possible within November?

[llogiq]

I finally found the time to review this. Good job and thank you!