Skip to content

Let CI run Python unit tests#463

Merged
mkcor merged 1 commit into
masterfrom
python-unit-tests
Jan 17, 2020
Merged

Let CI run Python unit tests#463
mkcor merged 1 commit into
masterfrom
python-unit-tests

Conversation

@mkcor

@mkcor mkcor commented Jan 16, 2020

Copy link
Copy Markdown
Contributor

Closes #462.

About

  • This is a new component
  • I am adding a feature to an existing component, or improving an existing feature
  • I am closing an issue

Description of changes

I was quite tempted to add a Python command right next to python unit_test_data_setup.py. Instead, should we separate Python unit tests and JS unit tests into two different steps?
Oops, that step of running Python unit tests definitely can't be part of a node job! I have rebased.

Before merging

@shammamah-zz
shammamah-zz temporarily deployed to dash-bio-test-pr-463 January 16, 2020 17:23 Inactive
@shammamah-zz
shammamah-zz temporarily deployed to dash-bio-test-pr-463 January 16, 2020 17:30 Inactive
@shammamah-zz

Copy link
Copy Markdown
Contributor

should we separate Python unit tests and JS unit tests into two different steps?

I don't think so. The unit tests are meant to test components individually. Some of those components are written in Python instead of JS, but that doesn't mean they're two different types of tests.

@mkcor

mkcor commented Jan 16, 2020

Copy link
Copy Markdown
Contributor Author

should we separate Python unit tests and JS unit tests into two different steps?

I don't think so. The unit tests are meant to test components individually. Some of those components are written in Python instead of JS, but that doesn't mean they're two different types of tests.

Oops, I have rebased and edited the description (see above) before seeing your comment... I agree they are both unit tests, but since the CI config is structured into node vs Python jobs, jobs being two levels above steps... it feels wrong not to have this pytest command under the Python jobs (i.e., 'where it belongs').

@Marc-Andre-Rivet Marc-Andre-Rivet left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

💃 LGTM. One tiny bit more sanity.

I have no preference as to whether the Py unit tests and the JS unit tests should be executed together. Dividing JS / Py seems ok for now. Some projects in Dash combine them, some don't, based on the current structure.

@mkcor

mkcor commented Jan 17, 2020

Copy link
Copy Markdown
Contributor Author

@shammamah by the way ;)

The unit tests are meant to test components individually.

This is not correct. Unit tests are meant to test code units (such as functions). The component level does not come into play; for convenience, we can use it in the file structure of course (e.g., file tests/unit/test_clustergram.py will gather tests for code units found in the Clustergram component). But the actual tests bear on code units.

@mkcor
mkcor merged commit d7896ca into master Jan 17, 2020
@mkcor
mkcor deleted the python-unit-tests branch January 31, 2020 21:02
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.

CI does not run Python unit tests.

3 participants