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 improper class.filename and package.name values on XML coverage report #863

Merged
merged 2 commits into from Aug 8, 2022

Conversation

lormico
Copy link
Contributor

@lormico lormico commented Jul 29, 2022

Solves #862

@arcivanov
Copy link
Member

flake8 warnings. @lormico

@lormico
Copy link
Contributor Author

lormico commented Jul 29, 2022

Whoooops, fixed it

@coveralls
Copy link

coveralls commented Jul 29, 2022

Pull Request Test Coverage Report for Build 2812061099

  • 2 of 2 (100.0%) changed or added relevant lines in 1 file are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage increased (+2.3%) to 84.672%

Totals Coverage Status
Change from base Build 2767987572: 2.3%
Covered Lines: 5241
Relevant Lines: 6140

💛 - Coveralls

@arcivanov
Copy link
Member

@lormico
Copy link
Contributor Author

lormico commented Jul 30, 2022

It looks like the Windows failures are not caused by my edit on coverage_plugin.py: I tried reverting my changes and disabling the assertions in the integration test, and it still fails when running the coverage step.

Maybe there's something wrong with GitHub's Windows testing platform? Should I make PyBuilder skip my test if "win" in sys.platform?

@arcivanov
Copy link
Member

No, it is not. It's the issue with disks/locations.

ValueError: path is on mount 'c:', start on mount 'D:'

@arcivanov
Copy link
Member

@lormico it's the change related to path manipulation. Current tree works fine:

https://github.com/pybuilder/pybuilder/runs/7593136238?check_suite_focus=true

@lormico
Copy link
Contributor Author

lormico commented Aug 1, 2022

Ok, I wanted to investigate deeper on this issue, so I created a branch on lormico/pybuilder from the latest commit on pybuilder/pybuilder/master and:

  1. removed all integration tests (merely for build speedup)
  2. added a new integration test which only sets up a simple project using the methods provided by itest_support.IntegrationTestSupport and then calls the coverage task (no assertions)

I still keep getting the same error, even if the src/main codebase is the same as the one on pybuilder/master, so I'd exclude that it's because of something that was introduced recently.

This particular error:

ValueError: path is on mount 'c:', start on mount 'D:'

seems to be due to how Python itself handles relative paths on Windows, when two different volumes are involved (here a very old ticket from 2009 which explains this better than I could do).

I'm still not completely sure about how this is connected to PyBuilder, but I'm more inclined to think that's because of how GitHub Actions work or how they are configured for a Windows VM. Alas, I have nearly zero experience with GitHub Actions, so I might be off track.

Do you have any pointers from here @arcivanov?

@arcivanov
Copy link
Member

@lormico I took another dig at it and it looks like a bug in coverage

nedbat/coveragepy#1428

@arcivanov
Copy link
Member

@lormico Please rebase on the the latest. The test also needs to be modified to not fail for unrelated reasons:

       File "d:\a\pybuilder\pybuilder\target\dist\pybuilder-0.13.7.dev\pybuilder\execution.py", line 95, in execute
        self.callable(*arguments)
        ^^^^^^^^^^^^^^^^^^^^^^^^^
      File "d:\a\pybuilder\pybuilder\target\dist\pybuilder-0.13.7.dev\pybuilder\plugins\python\coverage_plugin.py", line 157, in coverage
        raise failure
        ^^^^^^^^^^^^^
    pybuilder.errors.BuildFailedException: Test coverage for at least one module is below 70%
    
    ----------------------------------------------------------------------

Very close now 😄

@lormico
Copy link
Contributor Author

lormico commented Aug 8, 2022

I ran the Actions on my fork (I had to disable the upload step for pyb in order to make it run successfully, because of permissions), but during the build PyBuilder seems to have downloaded an old version of coverage, and failed on these two instances: [windows-latest, 3.11.0-beta.5, true, false] and [windows-latest, 3.11.0-beta.5, false, false].

The error reveals it is not using the latest version:

message(f"Combined data file {os.path.relpath(f)}")  # should be message(f"Combined data file {file_name}")

even though the dependency specification in pyb is coverage~=6.0, which should be ok.

I'm doing some tests to find out what is the cause.

@lormico
Copy link
Contributor Author

lormico commented Aug 8, 2022

Nevermind, it now doesn't seem to fail anymore.

Here is the Actions run on my fork.

@arcivanov can you please approve launching the workflow?

@arcivanov arcivanov merged commit 11d55c8 into pybuilder:master Aug 8, 2022
@arcivanov
Copy link
Member

@lormico Thank you, merged.

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.

None yet

3 participants