search

mrverdant13 / coverde

A set of commands for coverage trace files manipulation.
MIT License
30 stars 11 forks source link

Coverde filter using relative paths #68

Closed shane-henderson-308 closed 1 year ago

shane-henderson-308 commented 1 year ago

Prior to the release of 2.0, Coverde was using absolute paths for filter. This was essential for usage with melos when generating coverage reports so that the files could be found within the directory. Unfortunately the latest version of coverdeis now using relative paths, but these are relative to they module the reside in. So, for example, my filtered.lcov now has the following:

SF:lib/src/usecases/authentication/authentication_usecase.dart

This path is incorrect, because this file actually sits in my /domain module and as a result, trying to generate a report fails as the file cannot be found (since it's actually in domain/lib/src/usecases/authentication/authentication_usecase.dart).

In order to still be able to integrate with melos either absolute paths need to be used, or relative paths could be used, but they need to contain the module within the path too. All of our CircleCI builds are now failing because of this :(

mrverdant13 commented 1 year ago

Hello, @shane-henderson-308 ! 👋🏼 You are right!

That was totally my bad 😖 since there were no tests checking this behaviour against mono-repo setups.

However, the use of absolute paths was also leading to some cross-platform issues.

Potential solution

My first thought is that the filter command could include an optional --paths-parent option that would serve as prefix for all source file paths in the lcov file.

Example

Inputs

--paths-parent option:

parent/folder/path/

Input trace file:

SF:lib\core\config.dart
...
SF:lib\core\config.g.dart
...
SF:lib\core\dependency_injection.dart
...
SF:lib\core\flavors.dart
...

Output

SF:parent\folder\path\lib\core\config.dart
...
SF:parent\folder\path\lib\core\config.g.dart
...
SF:parent\folder\path\lib\core\dependency_injection.dart
...
SF:parent\folder\path\lib\core\flavors.dart
...

melos setup

With that proposal, the corresponding lcov-merging script would be as follows:

M:
  description: Merge all packages coverage trace files ignoring data related to generated files.
  run: >
    coverde rm MELOS_ROOT_PATH/coverage/filtered.lcov.info &&
    melos exec --file-exists=coverage/lcov.info -- "coverde filter --input ./coverage/lcov.info --output MELOS_ROOT_PATH/coverage/filtered.lcov.info --filters '\.g\.dart' --paths-parent MELOS_PACKAGE_PATH"

As you can see, the merged trace file would be placed in the mono-repo root dir and every referenced path within the trace file of each package would be properly prefixed to match the project structure by taking advantage of the melos MELOS_PACKAGE_PATH env variable.

I believe this approach would address the bug you just reported and would not result in any other undesired side effects.

Let me know what do you think about it.

shane-henderson-308 commented 1 year ago

Thank you so much for your extraordinarily fast reply. Yes, that approach would definitely work. I assumed that there must have been a reason for removing absolute paths as a bug fix, so I was hoping there could be a suitable alternative, and this sounds like it!

On Wed, 16 Aug 2023 at 13:49, Karlo Verde @.***> wrote:

Hello, @shane-henderson-308 https://github.com/shane-henderson-308 ! 👋🏼 You are right!

That was totally my bad 😖 since there were no tests checking this behaviour against mono-repo setups.

However, the use of absolute paths was also leading to some cross-platform issues.

Potential solution

My first thought is that the filter command could include an optional --paths-parent option that would serve as prefix for all source file paths in the lcov file. Example Inputs

--paths-parent option:

parent/folder/path/

Input trace file:

SF:lib\core\config.dart ... SF:lib\core\config.g.dart ... SF:lib\core\dependency_injection.dart ... SF:lib\core\flavors.dart ...

Output

SF:parent\folder\path\lib\core\config.dart ... SF:parent\folder\path\lib\core\config.g.dart ... SF:parent\folder\path\lib\core\dependency_injection.dart ... SF:parent\folder\path\lib\core\flavors.dart ...

melos setup

With that proposal, the corresponding lcov-merging script would be as follows:

M: description: Merge all packages coverage trace files ignoring data related to generated files. run: > coverde rm MELOS_ROOT_PATH/coverage/filtered.lcov.info && melos exec --file-exists=coverage/lcov.info -- "coverde filter --input ./coverage/lcov.info --output MELOS_ROOT_PATH/coverage/filtered.lcov.info --filters '.g.dart' --paths-parent MELOS_PACKAGE_PATH"

As you can see, the merged trace file would be placed in the mono-repo root dir and every referenced path within the trace file of each package would be properly prefixed to match the project structure by taking advantage of the melos MELOS_PACKAGE_PATH env variable.

I believe that approach would address the bug you just reported and would not result in any other undesired side effects.

Let me know what do you think about it.

— Reply to this email directly, view it on GitHub https://github.com/mrverdant13/coverde/issues/68#issuecomment-1679934467, or unsubscribe https://github.com/notifications/unsubscribe-auth/AWDG74YWKVVQPBIJB3CKCMTXVRC3RANCNFSM6AAAAAA3R4643A . You are receiving this because you were mentioned.Message ID: @.***>

--

CAUTION: This email, links and files included in its transmission by Woolworths Group Limited ABN 88 000 014 675 and its group of companies (Woolworths Group) are solely intended for the use of the addressee(s) and may contain information that is confidential and privileged. If you receive this email in error, please advise us immediately and delete it without reading or copying the contents contained within. Woolworths Group does not accept liability for the views expressed within or the consequences of any computer malware that may be transmitted with this email. The contents are also subject to copyright. No part of it should be reproduced, adapted or transmitted without the written consent of the copyright owner.

mrverdant13 commented 1 year ago

This has been indirectly addressed by #69 Still working on some docs updates before publishing the new version

mrverdant13 commented 1 year ago

A new version is now available on the Pub.dev And the errored version has been retracted.

shane-henderson-308 commented 1 year ago

Thank you!

On Fri, 18 Aug 2023 at 5:30 pm, Karlo Verde @.***> wrote:

A new version is now available on the Pub.dev And the errored version has been retracted.

— Reply to this email directly, view it on GitHub https://github.com/mrverdant13/coverde/issues/68#issuecomment-1683522263, or unsubscribe https://github.com/notifications/unsubscribe-auth/AWDG74ZR6NPHJ44SUK7PF4LXV4OK5ANCNFSM6AAAAAA3R4643A . You are receiving this because you were mentioned.Message ID: @.***>

--

CAUTION: This email, links and files included in its transmission by Woolworths Group Limited ABN 88 000 014 675 and its group of companies (Woolworths Group) are solely intended for the use of the addressee(s) and may contain information that is confidential and privileged. If you receive this email in error, please advise us immediately and delete it without reading or copying the contents contained within. Woolworths Group does not accept liability for the views expressed within or the consequences of any computer malware that may be transmitted with this email. The contents are also subject to copyright. No part of it should be reproduced, adapted or transmitted without the written consent of the copyright owner.

shane-henderson-308 commented 1 year ago

Hi Karlo

I'm so sorry to bother you... I have installed the latest version of coverde and I have added melos scripts as you suggested. Below is what I have:

melos exec --file-exists=coverage/lcov.info -- coverde filter -i ./coverage/ lcov.info -o MELOS_ROOT_PATH/coverage/base.lcov.info --filters='.g.dart' --paths-parent=MELOS_PACKAGE_PATH coverde report -i MELOS_ROOT_PATH/coverage/base.lcov.info --medium 50 --high 80

I can see that the my base.lcov.info file now has paths like so:

SF:/Users/shane/Woolworths/WooliesGO/driver_app_flutter/domain/lib/src/models/configuration/driver_break_timer.dart DA:7,0 DA:15,0 DA:16,0 LF:3 LH:0 end_of_record SF:/Users/shane/Woolworths/WooliesGO/driver_app_flutter/domain/lib/src/models/configuration/live_location.dart DA:7,0 DA:21,0 DA:23,0 DA:24,0 DA:26,0 LF:5 LH:0 end_of_record

But when it runs "coverde report -i MELOS_ROOT_PATH/coverage/base.lcov.info --medium 50 --high 80" I still see errors? I'm seeing "PathNotFoundException: Cannot open file, path = 'Users/shane/Woolworths/WooliesGO/driver_app_flutter/domain/lib/src/models/configuration/driver_break_timer.dart' (OS Error: No such file or directory, errno = 2)"

The part that confuses me is that those files are definitely there in that directory. I can see them! I was hoping you might be able to help? Coverde is awesome - we would love to keep using it!

Thanks Shane

On Fri, 18 Aug 2023 at 17:30, Karlo Verde @.***> wrote:

A new version is now available on the Pub.dev And the errored version has been retracted.

— Reply to this email directly, view it on GitHub https://github.com/mrverdant13/coverde/issues/68#issuecomment-1683522263, or unsubscribe https://github.com/notifications/unsubscribe-auth/AWDG74ZR6NPHJ44SUK7PF4LXV4OK5ANCNFSM6AAAAAA3R4643A . You are receiving this because you were mentioned.Message ID: @.***>

--

CAUTION: This email, links and files included in its transmission by Woolworths Group Limited ABN 88 000 014 675 and its group of companies (Woolworths Group) are solely intended for the use of the addressee(s) and may contain information that is confidential and privileged. If you receive this email in error, please advise us immediately and delete it without reading or copying the contents contained within. Woolworths Group does not accept liability for the views expressed within or the consequences of any computer malware that may be transmitted with this email. The contents are also subject to copyright. No part of it should be reproduced, adapted or transmitted without the written consent of the copyright owner.

mrverdant13 commented 1 year ago

Hey again, @shane-henderson-308 👋🏼

Well, I feel the issue is related to the prefix path. I believe you are on a Windows PC with WSL, right? The Users/ folder should be under a root drive, e.g. C:/, D:/, or so.

If that is not the case, I would appreciate it if you could give more context on your OS, Dart version, or any additional details that might be handy to consider when debugging this.

PS1: The coverde CLI itself uses melos, you can check the melos.yaml file in the root as reference. PS2: Please consider also providing formatted code.

shane-henderson-308 commented 1 year ago

Hi Karlo

Absolutely - anything I can provide you with which might help, please just ask.

I am using a: MacBook Pro (M2) running 13.4.1 (c).

My dart version is: Dart SDK version: 3.0.0 (stable) (Thu May 4 01:11:00 2023 -0700) on "macos_arm64"

The path I'm using is a pretty standard Mac path, which is: /Users/shane/Woolworths/WooliesGO/driver_app_flutter

I have attached the melos.yaml file and melos_driver_app_flutter.iml in case either of those help.

As I said, please let me know if there is anything else I can do to try and help.

Shane

On Mon, 21 Aug 2023 at 13:34, Karlo Verde @.***> wrote:

Hey again, @shane-henderson-308 https://github.com/shane-henderson-308 👋🏼

Well, I feel the issue is related to the prefix path. I believe you are on a Windows PC with WSL, right? The Users/ folder should be under a root drive, e.g. C:/, D:/, or so.

If that is not the case, I would appreciate it if you could give more context on your OS, Dart version, or any additional details that might be handy to consider when debugging this.

PS: The coverde CLI itself uses melos, you can check the melos.yaml file in the root as reference.

— Reply to this email directly, view it on GitHub https://github.com/mrverdant13/coverde/issues/68#issuecomment-1685607094, or unsubscribe https://github.com/notifications/unsubscribe-auth/AWDG743ZVAQJQPAPIDQXU6DXWLM5JANCNFSM6AAAAAA3R4643A . You are receiving this because you were mentioned.Message ID: @.***>

--

CAUTION: This email, links and files included in its transmission by Woolworths Group Limited ABN 88 000 014 675 and its group of companies (Woolworths Group) are solely intended for the use of the addressee(s) and may contain information that is confidential and privileged. If you receive this email in error, please advise us immediately and delete it without reading or copying the contents contained within. Woolworths Group does not accept liability for the views expressed within or the consequences of any computer malware that may be transmitted with this email. The contents are also subject to copyright. No part of it should be reproduced, adapted or transmitted without the written consent of the copyright owner.