Skip to content

Commit

Permalink
added coverage file name to Coveralls action
Browse files Browse the repository at this point in the history
  • Loading branch information
@jan-karstens-by
jan-karstens-by committed May 20, 2020
1 parent 3253c85 commit 47e90be
Showing 1 changed file with 1 addition and 0 deletions.
1 change: 1 addition & 0 deletions .github/workflows/main.yml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,7 @@ jobs:
uses: coverallsapp/github-action@master
with:
github-token: ${{ secrets.github_token }}
path-to-lcov: .coverage
flag-name: run-python_${{ matrix.python }}-${{ matrix.connector }}
parallel: true

Expand Down

11 comments on commit 47e90be

@tkilias
Copy link
Collaborator

Choose a reason for hiding this comment

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

@BY-jk maybe a problem with the version of coveralls, currently we don't specify one while installing it. We can also try to add and pip freeze. I saw it the travis config, but forget to add it for the github actions.

@tkilias
Copy link
Collaborator

Choose a reason for hiding this comment

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

And, when I look at the action definition of coverall, maybe we should use a release version and not master.

@tkilias
Copy link
Collaborator

@tkilias tkilias commented on 47e90be May 20, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

@BY-jk hm, this looks like the problem coverallsapp/github-action#49

In the last comment, someone said:

PS: also strange that this repo support only lcov files and don't accept standard json, for example python's coverage generated by pytest isn't pushed using this GitHub action -> lcov file not found.

@jank
Copy link

@jank jank commented on 47e90be May 20, 2020

Choose a reason for hiding this comment

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

The issue I ran into with coveralls was that pytest-cov produces a .coverage file (this is an SQLite DB file) but it is looking for an lcov.info file with a different format. It was far from obvious to me how to configure which action or which tool to produce the right output.
So I removed coverage reporting for now to make the four parallel test run.
Priority right now is to fix the broken turbodbc tests on Python 2.7: https://github.com/blue-yonder/sqlalchemy_exasol/runs/692399753?check_suite_focus=true

@tkilias
Copy link
Collaborator

Choose a reason for hiding this comment

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

@BY-jk Regarding turbodbc, I might found the cause. There is a new major version since 8 days and maybe that changes things. The Easiest test would be, to pin the version of the dependency to an earlier version.
https://github.com/blue-yonder/turbodbc/releases

@jank
Copy link

@jank jank commented on 47e90be May 20, 2020

Choose a reason for hiding this comment

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

@BY-jk Regarding turbodbc, I might found the cause. There is a new major version since 8 days and maybe that changes things. The Easiest test would be, to pin the version of the dependency to an earlier version.
https://github.com/blue-yonder/turbodbc/releases

All old releases prior 4.0.0 have been dropped from PyPi. Turbodbc 4.0.0 only supports Python 3.6 onward. Need to investigate.

@tkilias
Copy link
Collaborator

Choose a reason for hiding this comment

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

So, at least I could confirm that the turbodbc 3.3.0 works with SQLA on Python 2.7
https://github.com/tkilias/sqlalchemy_exasol/tree/patch-1
Maybe we need a Turbodbc version switch in the CI depending on the Python version and could explain in the Readme that SQLA on Python 2.7 only compatible with Turbodbc <= 4.0.0 is.

@jank
Copy link

@jank jank commented on 47e90be May 20, 2020

Choose a reason for hiding this comment

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

Yes, I have pinned to 3.3.0 for now to get back to a stable build. Adding coverage and upload to PyPi should have priority now so that we can merge the #90.
Next would be the decision on how long to continue to support 2.7. An alternative is to release 3.0.0 of this dialect that drops 2.7 support. It's dead anyhow.

@tkilias
Copy link
Collaborator

Choose a reason for hiding this comment

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

@BY-jk maybe that helps with the coverage
coverallsapp/github-action#30 (comment)

@jank
Copy link

@jank jank commented on 47e90be May 20, 2020

Choose a reason for hiding this comment

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

This could help but is currently in development state. To make progress and dump Travis I suggest to focus on uploading to PyPi on successful builds with tags. I will talk to @nndo1991 to look into this.

@pbelskiy
Copy link

Choose a reason for hiding this comment

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

Action you use supports only lcov format which isn't produced by pytest

Please sign in to comment.