Skip to content

GitHub Action for Linux Unit Tests#2013

Merged
ParthivNaresh merged 66 commits intomainfrom
1911-Linux-Unit-Tests-Action
Apr 5, 2021
Merged

GitHub Action for Linux Unit Tests#2013
ParthivNaresh merged 66 commits intomainfrom
1911-Linux-Unit-Tests-Action

Conversation

@ParthivNaresh
Copy link
Contributor

@ParthivNaresh ParthivNaresh commented Mar 22, 2021

Fixes #1911

Using this to close out #1911 which was raised to find a way to upload a codecov report for test coverage.
Here is the Linux Unit Test GitHub Action: https://github.com/alteryx/evalml/runs/2254366693?check_suite_focus=true
Here's the uploaded report: https://codecov.io/gh/alteryx/evalml/tree/9bdba6c299d517dab05ecc35dd79f0272148333b/evalml

For comparative review, here is a PR raised to see how this Action behaves when a large number of model_understanding tests have been removed, thereby reducing code coverage: #2012

@ParthivNaresh ParthivNaresh marked this pull request as ready for review April 2, 2021 15:28
Copy link
Contributor

@freddyaboulton freddyaboulton left a comment

Choose a reason for hiding this comment

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

@ParthivNaresh Awesome! This is super exciting. I have some questions on the implementation but nothing blocking.

- if: ${{ matrix.core_dependencies }}
name: Installing Core Dependencies
run: |
pip install virtualenv
Copy link
Contributor

Choose a reason for hiding this comment

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

How come we install virtualenv in every step?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I was running into some errors a while back about the virtualenv not being carried forward between steps but I'll try again and see what happens.

- python_version: "3.8"
core_dependencies: false
codecov: true
- python_version: "3.8"
Copy link
Contributor

Choose a reason for hiding this comment

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

I think we're missing python 3.9 core dependecies False?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Great point thanks!

Copy link
Contributor

Choose a reason for hiding this comment

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

Didn't see the core_dependencies=False for 3.9

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@bchen1116 My total bad, great catch

nbval==0.9.3
IPython>=5.0.0
codecov==2.1.0
codecov==2.1.11
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this version needed to upload the report?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

2.1.6 adds more support for GitHub Actions and 2.1.11 adds Python 3.9 support!

Copy link
Contributor

Choose a reason for hiding this comment

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

noice

pip install --upgrade pip -q
pip install -e . --no-dependencies
pip install -r core-requirements.txt
pip install -r test-requirements.txt
Copy link
Contributor

Choose a reason for hiding this comment

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

I can't remember why we didn't create a Makefile target for installing coredeps, like make installdeps-core 🤷‍♂️

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'll add one!

pip install -r core-requirements.txt
pip install -r test-requirements.txt
# "!" negates return code. exit nonzero if any of these deps are found
! pip freeze | grep -E "xgboost|catboost|lightgbm|plotly|ipywidgets|category_encoders"
Copy link
Contributor

Choose a reason for hiding this comment

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

Do you think this check will still work for GH actions instead of circleci?

Quick test would be: create a do-not-merge copy of your PR, add pip install lightgbm above this line and make sure the job fails on that PR.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Copy link
Contributor

@dsherry dsherry left a comment

Choose a reason for hiding this comment

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

LGTM! Lmk when you're ready to update branch protections @ParthivNaresh ⚔️ 🤺

Copy link
Contributor

@bchen1116 bchen1116 left a comment

Choose a reason for hiding this comment

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

LGTM! Thanks for updating and cleaning our test code!

- python_version: "3.8"
core_dependencies: false
codecov: true
- python_version: "3.8"
Copy link
Contributor

Choose a reason for hiding this comment

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

Didn't see the core_dependencies=False for 3.9

virtualenv test_python -q
source test_python/bin/activate
pip install coverage
bash <(curl -s https://codecov.io/bash) -C ${{ github.event.pull_request.head.sha }}
Copy link
Contributor

Choose a reason for hiding this comment

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

Ah nice, so this was the SHA which fixed the issue. Sweet!

.PHONY: installdeps-core
installdeps-core:
pip install -e . -q
pip install -r core-requirements.txt -q
Copy link
Contributor

Choose a reason for hiding this comment

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

🙏

@ParthivNaresh ParthivNaresh merged commit 8df4215 into main Apr 5, 2021
@chukarsten chukarsten mentioned this pull request Apr 6, 2021
@freddyaboulton freddyaboulton deleted the 1911-Linux-Unit-Tests-Action branch May 13, 2022 15:18
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.

Codecov GitHub Action unable to upload a report for test coverage

5 participants