Consider running tests **without** `src/` changes, to validate that they must **fail**

[Ocramius]

RFC

Q	A
Proposed Version(s)	?
BC Break?	No

Goal

We want to verify that the tests accompanying a source change do validate the source change.

Background

This was discussed with @ondrejmirtes in private chat.

When receiving a source change, the accompanying tests must be verified for their quality.

In order to do that, we could:

1. checkout the changes
2. reset source(s) (based on composer.json paths) to their pre-patch state
3. run the tests, expect them to fail

Considerations

• what to do when only tests are changed?
• what to do when only sources are changed?
• what to do when composer.lock changed together with the tests?

Proposal(s)

1. we will need a CLI utility that verifies this behavior inside the continuous-integration container
2. we will need to adjust the CI matrix action to generate the appropriate verification job
3. we only operate on locked dependencies

[ondrejmirtes]

Two exceptions I realized that should be applied here:

1) When the changes in src/ only remove code, it's possible that tests will not fail when the changes are reverted. For example we can remove untested dead code this way. 2) When the changes in src/ are about code style only, tests will not fail when the changes are reverted.

It should be possible to detect 1) easily (0 added lines, X removed lines), but it's probably not possible to detect 2) reliably.

[staabm]

2) could be detected by running the "before-state" of a PR thru a CS tool and the "after-state". when comparing the results both CS differences should be normalized

[rajyan]

This seems difficult than it looks. For example in https://github.com/phpstan/phpstan-src/pull/1934/files I added a failing test https://github.com/phpstan/phpstan-src/pull/1934/files#r1008005278 and all other test cases were regression tests related to the code that was not covered.

I think the only way to know the intension of the commit is to express whether the test is a "failing test" or a "regression test" in the commit message or something.

so I can think of a CI that

• Verify test addition/modification commit’s commit message includes some keywords to distinguish the test type
• Run test with src/ reset and verify the results by test types

[Ocramius]

Note: the composer.json autoload section should be used to determine what to git reset.

[boesing]

Not 100% sure what this is about. Do we talk about something like:

git diff --staged ${TARGET_BRANCH_REF} src/ > pr.patch
git checkout ${TARGET_BRANCH_REF}
git apply pr.patch
vendor/bin/phpunit

[Xerkus]

more like

git checkout "${GITHUB_REF}"
git checkout "${GITHUB_BASE_REF}" -- $(get_src_paths_from_composer())
vendor/bin/phpunit # fail if this does not fail

[staabm]

See the linked PR above

https://github.com/phpstan/phpstan-src/pull/2029