Skip to content
This repository has been archived by the owner on Oct 12, 2022. It is now read-only.

Fix Issue 14790: rt: cover: coverage merge should detect changed source code #3341

Merged
merged 2 commits into from
Jan 27, 2021

Conversation

ljmf00
Copy link
Member

@ljmf00 ljmf00 commented Jan 23, 2021

Merging different versions of the same source file could lead to mismatched
covered lines if the source code actually changes. Changing the .lst file
format will open a huge breaking change and rely on timestamps is impractical.

As an alternative to this, we can deeply compare the source code with generated
.lst which also has a copy of the original source code.

Signed-off-by: Luís Ferreira contact@lsferreira.net

This test case was forgotten and not being tested and should be.

Signed-off-by: Luís Ferreira <contact@lsferreira.net>
@dlang-bot
Copy link
Contributor

Thanks for your pull request and interest in making D better, @ljmf00! We are looking forward to reviewing it, and you should be hearing from a maintainer soon.
Please verify that your PR follows this checklist:

  • My PR is fully covered with tests (you can see the coverage diff by visiting the details link of the codecov check)
  • My PR is as minimal as possible (smaller, focused PRs are easier to review than big ones)
  • I have provided a detailed rationale explaining my changes
  • New or modified functions have Ddoc comments (with Params: and Returns:)

Please see CONTRIBUTING.md for more information.


If you have addressed all reviews or aren't sure how to proceed, don't hesitate to ping us with a simple comment.

Bugzilla references

Auto-close Bugzilla Severity Description
14790 enhancement coverage merge should detect changed source code

Testing this PR locally

If you don't have a local development environment setup, you can use Digger to test this PR:

dub run digger -- build "master + druntime#3341"

@dlang-bot dlang-bot added the Enhancement New functionality label Jan 23, 2021
@ljmf00 ljmf00 force-pushed the fix-merge-coverage branch from f7222cd to fbd188d Compare January 23, 2021 17:12
@ljmf00 ljmf00 marked this pull request as draft January 23, 2021 17:26
@ljmf00 ljmf00 force-pushed the fix-merge-coverage branch from fbd188d to 74e2094 Compare January 25, 2021 14:38
@ljmf00 ljmf00 marked this pull request as ready for review January 25, 2021 14:53
@ljmf00
Copy link
Member Author

ljmf00 commented Jan 25, 2021

buildkite is heisenberging ¯\_(ツ)_/¯

…ce code

Merging different versions of the same source file could lead to mismatched
covered lines if the source code actually changes. Changing the .lst file
format will open a huge breaking change and rely on timestamps is impractical.

As an alternative to this, we can deeply compare the source code with generated
.lst which also has a copy of the original source code.

Signed-off-by: Luís Ferreira <contact@lsferreira.net>
@ljmf00 ljmf00 force-pushed the fix-merge-coverage branch from 74e2094 to 6c7dc69 Compare January 25, 2021 18:58
@12345swordy
Copy link
Contributor

cc @RazvanN7 can you help with this?

@dlang-bot dlang-bot merged commit f96b1b0 into dlang:master Jan 27, 2021
@ghost
Copy link

ghost commented Mar 14, 2021

This causes a regression. Sometimes that corrupts the input lst. see https://issues.dlang.org/show_bug.cgi?id=21712.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Enhancement New functionality
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants