Skip to content
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

Fix isChild check in CoberturaMultiSourceReader #266

Merged
merged 1 commit into from
Jul 9, 2023

Conversation

ruippeixotog
Copy link
Contributor

591ff35 changed isChild to compare absolute paths rather than relative ones. However, it did something else - changed the variables being compared from instances of Path to Strings. While startsWith is defined in both types, their behavior is not the same. Path#startsWith correctly determines if a path is a parent of another because it takes file path elements in consideration, while String#startsWith does prefix checks at a character level.

In real scenarios, this fails when using Scala version specific source folders like src/main/scala and src/main/scala-2.12, natively supported by SBT. They are sibling directories, but the current implementation incorrectly assumes them to be parent and children, respectively. On this PR we're fixing it by keeping usage of absolute paths while still using Path.

Added a unit test that correctly catches this error so that hopefully we don't get any regressions.

@ruippeixotog
Copy link
Contributor Author

@rolandtritsch, do you mind retrying the test job and reviewing the PR when you have time? I believe CI failed due to a flaky test on a sample project.

@rolandtritsch
Copy link
Member

Hi @ruippeixotog. Taking a look now ...

Copy link
Member

@rolandtritsch rolandtritsch left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good. Thank you. Merging now.

@rolandtritsch rolandtritsch merged commit d9035ff into scoverage:main Jul 9, 2023
@rolandtritsch
Copy link
Member

Hi @ruippeixotog. Thank you for the fix/PR. Published a new release with the fix. - https://github.com/scoverage/sbt-coveralls/releases/tag/v1.3.9

@ruippeixotog ruippeixotog deleted the fix_is_child_check branch July 9, 2023 17:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants