search

MOxUnit / MOcov

A coverage report generator for Matlab and GNU Octave.
20 stars 12 forks source link

Extra warning and errors when using on Windows #28

Closed Remi-Gau closed 1 year ago

Remi-Gau commented 1 year ago

Currently implementing to use moxunit and mocov in CI on github and run my tests with matlab on ubuntu, macos and windows.

See this PR on a template repo: https://github.com/Remi-Gau/template_matlab_analysis/pull/23

In CI on windows I am getting extra warnings and errors when asking for coverage that I do not get:

A typical warning would look something like this:

https://github.com/Remi-Gau/template_matlab_analysis/actions/runs/4860625025/jobs/8664661302?pr=23#step:5:19

   [Warning: Escaped character '\g' is not valid. See 'doc sprintf' for supported special characters.] 
  [> In MOcovMFileCollection.rewrite_mfiles>@(line_number)sprintf(pat,line_number) (line 36)
  In MOcovMFile/write_lines_with_prefix (line 32)
  In MOcovMFileCollection/rewrite_mfiles (line 37)
  In MOcovMFileCollection/prepare (line 44)
  In mocov (line 92)
  In moxunit_runtests>run_all_tests (line 205)
  In moxunit_runtests (line 102)
  In run_tests (line 21)
  In run (line 91)
  In run_tests_ci (line 12)
  In run (line 91)
  In command_b91aebef_4fe2_4a34_b8f8_9545a323cf1a (line 1)
  In mdcsbatch (line 58)]

extra errors are usually of the type:

https://github.com/Remi-Gau/template_matlab_analysis/actions/runs/4860625025/jobs/8664661302?pr=23#step:5:142

  failure: Error: File: C:\Users\runneradmin\AppData\Local\Temp\tpaffcc5ec_1cd7_485e_86ca_7f00afc53b35\utils\is_octave.m Line: 12 Column: 22
  Character vector is not terminated properly.
    test_my_fibonacci_error:39 (D:\a\template_matlab_analysis\template_matlab_analysis\tests\test_my_fibonacci.m)

From a quick look it at least seems that the warning happen when the tested file is in a subfolder: windows path separator \ does not play well with sprintf.

Not yet sure what cause the errors though.

I will keep on investigating the possible causes and update this issue.

nno commented 1 year ago

Thanks for the report. On first impression I don't see what is causing this issue. If you have any updates / insights I would appreciate it if you reported here (or sent a PR).

nno commented 1 year ago

One idea: could you try mocov_get_absolute_path and mocov_get_relative_path and see if, using Windows, they behave as expected? The code as developed on a unix-like OS (and I don't have access to a Windows platform), so it is possible that on Windows there is some unexpected behaviour.

Remi-Gau commented 1 year ago

I can try to PR this and set up some CI on github for the mocov repo to prevent an eventual regression in the future.

Sent from Outlook for Androidhttps://aka.ms/AAb9ysg

From: Nikolaas N. Oosterhof @.> Sent: Wednesday, May 3, 2023 1:52:50 PM To: MOcov/MOcov @.> Cc: Remi Gau @.>; Author @.> Subject: Re: [MOcov/MOcov] Extra warning and errors when using on Windows (Issue #28)

One idea: could you try mocov_get_absolute_path and mocov_get_relative_path and see if, using Windows, they behave as expected? The code as developed on a unix-like OS (and I don't have access to a Windows platform), so it is possible that on Windows there is some unexpected behaviour.

— Reply to this email directly, view it on GitHubhttps://github.com/MOcov/MOcov/issues/28#issuecomment-1533468370, or unsubscribehttps://github.com/notifications/unsubscribe-auth/ABVDQINS4MA3IRKPB4OB2F3XEKLPFANCNFSM6AAAAAAXS7GNAA. You are receiving this because you authored the thread.Message ID: @.***>

Remi-Gau commented 1 year ago

OK I confirmed at least in github CI on windows that the extra warnings and errors appear when there are nested folders in the src folder to cover.

this run worked: https://github.com/Remi-Gau/MOcov/actions/runs/4941609719/jobs/8834386824

this one failed: https://github.com/Remi-Gau/MOcov/actions/runs/4941635683/jobs/8834441491

after moving a function into a subfolder: https://github.com/Remi-Gau/MOcov/pull/1/commits/9aa57206b48f8b96c31241ae99de7b6a0b8556f8

nno commented 1 year ago

@Remi-Gau Can you see if the branch in this PR fixes the issue?

https://github.com/MOcov/MOcov/pull/30

Remi-Gau commented 1 year ago

Woohoo ! 🚀

I merged your source branch of the #30 into mine for #29 and now all tests on my CI pass without errors or warning or windows.

See here: https://github.com/Remi-Gau/MOcov/actions/runs/4947108982/jobs/8846052514

So as far as I concerned I consider that as a fix.

Regarding #29 I could recycle it to leave to simply run some github-CI in windows to act as a regression test. Let me know what you think.

nno commented 1 year ago

@Remi-Gau A simple regression test would be good. The smaller the better, I was thinking about just one test file in a subfolder.

I'm closing this issue as PR https://github.com/MOcov/MOcov/pull/30 seems to have addressed it.

Remi-Gau commented 1 year ago

OK will trim #29 to be the slimest. Will ping you when it's done.