-
-
Notifications
You must be signed in to change notification settings - Fork 154
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Use merge-base (three dot diff) instead of direct diff for git-diff-ase (#1653) #1654
Conversation
Documents change proposed in infection/infection#1654
hello
|
3588b86
to
8aee2c9
Compare
8aee2c9
to
c73935b
Compare
Thanks Maks. Would you mind taking a look? I added some temporary debugging code, which shows that the generated command line is
|
I didn't dig yet, but I think this is related to how GitHub clones the repo. Try locally to do the same as GitHub does, here:
then here infection/.github/workflows/mt-annotations.yaml Lines 50 to 51 in 4e21783
so, probably after such cloning, there won't be any diff by using |
ah right, that |
3ddabe7
to
a353a44
Compare
I think we can make it use the merge-base for comparison if available in the checkout, if not we can fall back to doing a direct diff between commits. Probably worth advising users to make CI servers do deep checkouts to ensure we can find the merge-base and don't mutate more (or occasionally less) than needed. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
cool, thank you @bdsl
@maks-rafalko Welcome, thank you too. Do you know when this is most likely to be in a release? |
It would be great if it can be on the 0.26 branch (maybe requires backport / cherry-pick ?) since we're still using PHP 7.4 |
if you want it to be released for 7.4, please open a new PR with cherry-picking it to 0.26 branch as a target and I will release it ASAP |
Will do, thanks. |
…se (infection#1653) (infection#1654) * Use merge-base (three dot diff) instead of direct diff for git-diff-base (infection#1653) * Add command line dumping for debugging * Fetch full history to allow finding common ancestor, not just last commit * Remove var_dumps * Debug with git depth 1 * Fall back to direct diff if merge base commit is not avialable * Restore deep git fetch * Kill UnwrapTrim mutant
PR for 0.26: #1655 |
#1655) * Use merge-base (three dot diff) instead of direct diff for git-diff-ase (#1653) (#1654) * Use merge-base (three dot diff) instead of direct diff for git-diff-base (#1653) * Add command line dumping for debugging * Fetch full history to allow finding common ancestor, not just last commit * Remove var_dumps * Debug with git depth 1 * Fall back to direct diff if merge base commit is not avialable * Restore deep git fetch * Kill UnwrapTrim mutant * Fix php parse error * Add test_it_falls_back_to_direct_diff_if_merge_base_is_not_availabe * Skip test on Windows
This PR:
Fixes #1653
Currently when using --git-diff-base=master infection generates mutants for any lines that appear in a direct diff between HEAD and master. This PR changes that to use diff between HEAD and the merge-base of HEAD and master - i.e. the diff you see on a github pull request.
We use the three dot diff syntax for git to get the comparison between the merge base and HEAD. This diff shows work done on the HEAD branch, and should ignore any changes made in the parallel on the base branch. This seems more useful for evaluating the head branch, and fits the description:
infection/src/Command/RunCommand.php
Line 253 in d96aa59