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: Fixed tests running more than once and minor improvements #1095

Open
wants to merge 1 commit into
base: develop
Choose a base branch
from
Open
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 10 additions & 13 deletions .github/workflows/main.yml
Original file line number Diff line number Diff line change
Expand Up @@ -24,34 +24,31 @@ jobs:
- uses: psf/black@stable

build:
name: Python==${{matrix.python}}
needs: lint
runs-on: ubuntu-latest
strategy:
matrix:
python: [3.7, 3.9]
python: [3.7, 3.8, 3.9]
Copy link
Member

Choose a reason for hiding this comment

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

Don't think this is required please check on comment on vaishnavi's pr

Copy link
Member Author

Choose a reason for hiding this comment

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

I added this because sometimes code blocks failed for particular software/library version. We should test each version that we provide support for.


steps:
- uses: actions/checkout@v2
- name: Set up Python 3.x
- name: Set up Python ${{ matrix.python }}
uses: actions/setup-python@v2
with:
python-version: ${{ matrix.python }}

- name: Install dependencies
run: |
python -m pip install --upgrade pip
if [ -f requirements.txt ]; then pip install -r requirements.txt; fi
- name: Run tests
run: |
python -m unittest discover tests -v
- name: Generate coverage report
run: |
pip install pytest
pip install pytest-cov
pytest --cov-config=.coveragerc
pytest --cov=./ --cov-report=xml
pip install -r requirements.txt

- name: Run tests and generate report
run: coverage run -m unittest discover tests -v

- name: Upload coverage to Codecov
uses: codecov/codecov-action@v1
if: ${{ matrix.python == 3.7}}
with:
file: ./coverage.xml
name: codecov-umbrella
Copy link
Member

@epicadk epicadk May 9, 2021

Choose a reason for hiding this comment

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

Probably not related to this issue but upload coverage only if the maxtrix version is 3.7. Can you make this change too ?

Copy link
Member Author

Choose a reason for hiding this comment

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

Do you want to upload coverage only when python version is 3.7?
If yes, then that can be done with a if block but why we need that?

Copy link
Member

Choose a reason for hiding this comment

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

Do you want to upload coverage only when python version is 3.7?
If yes, then that can be done with a if block but why we need that?

Because it's uploading the coverage for both the versions.

Copy link
Member Author

Choose a reason for hiding this comment

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

I think we should maintain coverage for all versions. May be in future we have some compatibility shims then coverage will change for versions

Copy link
Member

Choose a reason for hiding this comment

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

I don't see how the python version is going to affect the coverage : /. Plus When the bot comments there is no way to differentiate between which coverage corresponds to which version.

Copy link
Member Author

Choose a reason for hiding this comment

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

That was happening because all coverages have same name. Btw, I add a condition to upload coverage for Python 3.7

fail_ci_if_error: true