Improving Functional Test With `pytest` Fixtures #75

axiomcura · 2023-08-07T17:23:48Z

Description

Thank you for your contribution to pycytominer!
Please succinctly summarize your proposed change.
What motivated you to make this change?

Please also link to any relevant issues that your code is associated with.

This PR introduces using pytest fixtures

The main goal of this PR is to leverage fixtures to make functional test much easier. Below is a diagram on how fixtures and helper functions are implemented in a single test.

Simple diagram illustrating how functional tests work: starting for the right side, (red) is the fixture used to generate temporary testing directories per test, (yellow) is the functional test with all the steps done, (green) is the a helper function known as prepare_data() that does all the heavy lifting to transporting datasets into the testing folder.

The diagram illustrates the implementation of CytoSnake's functional tests. Initially, a fixture (depicted in red) serves as input for the functional test (depicted in yellow). Within this process, pytest identifies that the fixture includes a path where the test should occur, hence generating a temporary directory. Additionally, the fixture's teardown function ensures that pytest removes the testing directory once the test finishes. Overall, leveraging these fixtures eliminates the need for developers to manually create and delete temporary directories for every test.

Additionally, a new function called prepare_dataset() has been added. This function primarily serves to facilitate the easy selection and transfer of data to a testing directory. It requires just one input: the desired dataset's name. The dataset names correspond to the directory names located within cytosnake/tests/functional/dataset.

Additional changes

New module

New module created test_utils for storing general helper functions, making testing development much easier.

Documentation changes

The new test_utils module has now been added into the RTD documentation website source code

Future PR

A future pull request will cover the rationale, motivations, and the design process behind the implementation of functional tests in CytoSnake. This PR will also provide a standardized guide detailing how to effectively document functional tests on the Read the Docs (RTD) website.

What is the nature of your change?functional

Bug fix (fixes an issue).
Enhancement (adds functionality).
Breaking change (fix or feature that would cause existing functionality to not work as expected).
This change requires a documentation update.

Checklist

Please ensure that all boxes are checked before indicating that a pull request is ready for review.

I have read the CONTRIBUTING.md guidelines.
My code follows the style guidelines of this project.
I have performed a self-review of my own code.
I have commented my code, particularly in hard-to-understand areas.
I have made corresponding changes to the documentation.
My changes generate no new warnings.
New and existing unit tests pass locally with my changes.
I have added tests that prove my fix is effective or that my feature works.
I have deleted all non-relevant text in this pull request template.

MikeLippincott

Great PR. The documentation in the code looks amazing. The .cli.md files are above me so I can't provide much feedback... if you have time and could explain in person, that would be great! Thank you

cytosnake/tests/functional/test_cli.py

cytosnake/tests/test_utils.py

d33bs

Nice work! I left a few comments and suggestions throughout this review. Please don't hesitate to let me know if you have any questions or concerns.

I'm marking this as a "comment" review for now as I wasn't able to successfully run tests from the changes in this PR (running into a FileNotFoundError when running pytest for tests test_multiplates_with_multi_platemaps, test_one_plate_one_platemap, and test_multiplate_maps_no_barcode).

d33bs · 2023-08-08T14:15:50Z

cytosnake/tests/functional/datasets/standard_sqlite_single/metadata/platemap/platemap1.csv

Thinking about the future of CytoSnake testing, would it be useful to provide these datasets without the functional distinction in the directory path? For example, would these ever be used for unit or integration testing?

That's a good point. I think that's a great idea! When I was developing this, I had no idea how the datasets should be implemented because the unit and workflow tests were not developed yet.

cytosnake/tests/functional/datasets/standard_sqlite_single/plate_data1.sqlite

cytosnake/tests/__init__.py

cytosnake/tests/functional/test_cli.py

Co-authored-by: Dave Bunten <ekgto445@gmail.com>

axiomcura · 2023-08-10T16:39:44Z

@d33bs Mind giving this a second round of review?

Here are some changes that I have done:

I have moved the testing folder outside the module and placed it into the project root folder
I removed the zero based data files and added NF1 dataset
dataset folder is outside the functional test.
test_utils module has been moved into the utils section in CytoSnake
added more assertions for my robust testing

d33bs

Great work in addressing the comments and making changes! I've left a few more comments and generally felt things looked good. Please don't hesitate to let me know if you have any questions.

d33bs · 2023-08-11T15:24:08Z

tests/functional/pytest.ini

Consider removing this file and making use of the pyproject.toml file + style for this configuration. Using the pyproject.toml file would help reduce file/code additions and also help with test invocation from the project root directory.

done: 7b1291f

d33bs · 2023-08-11T15:53:30Z

tests/functional/test_cli.py

+    assert proc.returncode == 0
+    assert data_folder.exists()
+    assert cytosnake_file.exists()
+    assert barcodes_in_datafolder.exists()
+    assert metadata_in_datafolder.exists()
+    assert all([str(plate_data).endswith(".sqlite") for plate_data in all_plates])


Seeing how this pattern repeats, is there an opportunity for a common utility function for checking these things to help reduce code duplication?

done! f2cc176

d33bs · 2023-08-11T16:18:28Z

tests/functional/test_cli.py

+    assert proc.returncode == 0
+    assert data_folder.exists()
+    assert cytosnake_file.exists()
+    assert metadata_in_datafolder.exists()
+    assert all([str(plate_data).endswith(".sqlite") for plate_data in all_plates])


Given the addition of non-zero data for testing are there other more specific data-orientated tests which could be added here or elsewhere to help with validation? This could come in, for example, the form of data shape, types, unique values, etc.

Unfortunately, no, those tests will be done within the either unit testing or workflow testing. The functional tests just focus on emulating user experience with the CLI. For example, setting up your directory to be a project directory. No files are being open and read into memory in this step.

tests/functional/test_cli.py

d33bs · 2023-08-11T16:23:37Z

tests/functional/test_cli.py

+    """
+
+    # transfer data to testing folder
+    test_utils.prepare_dataset(test_data_name="nf1-data", test_dir_path=testing_dir)


Seeing how this pattern repeats, would this be a good opportunity for a fixture?

I was thinking about this earlier and I feel like the design of using fixture that require parameters are a bit daunting and may be not easy for new developers.

Below is an example if prepare_dataset() becomes a fixture:

@pytest.mark.parametrize("prepare_data", ["nf1"], indirect=True) def text_example(testing_dir, prepare_dataset) -> None: # testing code below

However, if a redesign of the testing framework is conducted, then I will consider this approach.

d33bs · 2023-08-11T16:28:30Z

tests/datasets/nf1-data-nobarcode/metadata/platemap/platemap_NF1_plate1.csv

Did this and the related files come from other locations (and include efforts from other people)? If so, consider adding a reference of some kind to help document their contributions to this project (for example, through a CITATION.cff file using the references key).

added: 1c6286d

tests/functional/pytest.ini

Co-authored-by: Dave Bunten <ekgto445@gmail.com>

axiomcura · 2023-08-11T20:09:27Z

@d33bs I have added comments and suggestions. I'll be merging until the end of the day if you do not have any more comments or suggestions.

Thank you!

axiomcura added 3 commits August 7, 2023 11:00

functional test update

a5bd47f

update rtd docs

5ef8e4d

changes in docs

b1aada6

axiomcura changed the title ~~Func tests~~ Improving Functional Test With pytest Fixtures Aug 7, 2023

axiomcura marked this pull request as ready for review August 7, 2023 19:40

axiomcura requested a review from MikeLippincott August 7, 2023 19:46

axiomcura added the enhancement New feature or request label Aug 7, 2023

MikeLippincott reviewed Aug 8, 2023

View reviewed changes

cytosnake/tests/functional/test_cli.py Outdated Show resolved Hide resolved

cytosnake/tests/test_utils.py Outdated Show resolved Hide resolved

applied pre-commit formatting

fc0614c

axiomcura requested a review from d33bs August 8, 2023 17:21

d33bs reviewed Aug 8, 2023

View reviewed changes

axiomcura and others added 14 commits August 8, 2023 14:06

Update cytosnake/tests/functional/test_cli.py

3c366b4

Co-authored-by: Dave Bunten <ekgto445@gmail.com>

Update cytosnake/tests/functional/test_cli.py

e177099

Co-authored-by: Dave Bunten <ekgto445@gmail.com>

moved helper functions into test_utils

cc439b5

added custom marks to tests

1cc1d23

removed extra cmd variable

8145a91

added pytest.raises

f550bfb

added more assertions for multi platemaps test

ee8bd55

update test

07a8e14

moved test to root, added nf1 data

209afaf

added test root path in test utils

6fa3d67

removed unwanted symlink

175a873

update data

e2381de

updated dataset and testing

cc6bb06

applied pre-commit formatting

3ad5d37

axiomcura requested a review from d33bs August 10, 2023 16:39

d33bs approved these changes Aug 11, 2023

View reviewed changes

axiomcura and others added 2 commits August 11, 2023 11:05

Update tests/functional/pytest.ini

79ba7d3

Co-authored-by: Dave Bunten <ekgto445@gmail.com>

removed ini file added pytest configs in pyproject

7b1291f

axiomcura added 3 commits August 11, 2023 13:57

added assertion function to check files

5fb4e0e

added pre-commit formatting

f2cc176

added citation for nf1 dataset

1c6286d

axiomcura merged commit bdd2339 into WayScience:main Aug 11, 2023

axiomcura deleted the func-tests branch August 24, 2023 21:53

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Improving Functional Test With `pytest` Fixtures #75

Improving Functional Test With `pytest` Fixtures #75

axiomcura commented Aug 7, 2023 •

edited

Loading

MikeLippincott left a comment

d33bs left a comment

d33bs Aug 8, 2023

axiomcura Aug 8, 2023

axiomcura commented Aug 10, 2023 •

edited

Loading

d33bs left a comment

d33bs Aug 11, 2023

axiomcura Aug 11, 2023

d33bs Aug 11, 2023

axiomcura Aug 11, 2023

d33bs Aug 11, 2023

axiomcura Aug 11, 2023

d33bs Aug 11, 2023

axiomcura Aug 11, 2023

d33bs Aug 11, 2023

axiomcura Aug 11, 2023

axiomcura commented Aug 11, 2023

Improving Functional Test With pytest Fixtures #75

Improving Functional Test With pytest Fixtures #75

Conversation

axiomcura commented Aug 7, 2023 • edited Loading

Description

Additional changes

Future PR

What is the nature of your change?functional

Checklist

MikeLippincott left a comment

Choose a reason for hiding this comment

d33bs left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

axiomcura commented Aug 10, 2023 • edited Loading

d33bs left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

axiomcura commented Aug 11, 2023

Improving Functional Test With `pytest` Fixtures #75

Improving Functional Test With `pytest` Fixtures #75

axiomcura commented Aug 7, 2023 •

edited

Loading

axiomcura commented Aug 10, 2023 •

edited

Loading