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

Adding Python Tests #121

Open
wants to merge 7 commits into
base: main
Choose a base branch
from
Open

Adding Python Tests #121

wants to merge 7 commits into from

Conversation

PrestonHager
Copy link
Contributor

@PrestonHager PrestonHager commented Nov 8, 2024

Added tests using the pytest module. They can be run with pytest tests in the root directory.

Only unit tests for the utility files have been added. A fixture for mongodb is included.

This branch does include the join_url from the url-fix branch which means this should only get merged if #120 is merged. Other than that, the files in this branch have no operational impacts and are only used for development purposes. There are currently 13 total tests for three different utility files.

Plans for the future:

  1. Write more tests for every other source file (as applicable)
  2. Include a requests fixture for any web related data
  3. Add regression tests so further pull requests can be easily verified

Duplicated the project information to the tool.poetry section so that
using poetry works.
Added shell.nix file which creates a virtual environment using poetry
and installs the necessary requirements.
Added requirements to the pyproject.toml file under the
tool.poetry.dependencies section.
Added poetry.lock file for reproducability on non-nix systems.
Updated README to include information about nix-shell environment.

Execute using `nix-shell` command.
When patching the MongoClient for tests, the patch is applied only to
lookup calls exactly matching pymongo.MongoClient so the import must be
changed to be exactly this.
Imports for join_url must come from openf1.util.misc.
Tests for the openf1 package using pytest.

Added unit tests for:
 - db utils
 - misc utils
 - type_casting utils

Added fixture for mongodb to use with database tests.

Added mongodb fixture data using first 10 entries of 2024 season.
@PrestonHager
Copy link
Contributor Author

#120 has been merged with a few conflicts, so conflicts were resolved.

Current there are only 13 unit tests.

3 tests fail because they expect utility functions to raise an error when bad data is put in (this shouldn't happen in the modules code, I think?)
Another test fails because I'm not sure what the actual return value of session_key_to_path is supposed to be since the _path attribute wasn't in any of the session data when I ran the module's ingestor.

Suggestions for branch management with tests:

  1. Create a test, dev, or similar named branch
  2. Merge this PR into that instead of main
  3. Setup CI/CD to run whenever a PR is made from the test branch to the main branch
  4. Maybe setup CI/CD to be manually activated for PRs to test branch from any other branch

Additionally, I didn't notice a linting file; should we also integrate pylint or a similar library into the test suite?

@br-g
Copy link
Owner

br-g commented Nov 23, 2024

Hello @PrestonHager,
Thank you for this contribution!

When I run the tests locally, 4 of them (out of 9) are failing. Is it the same for you?

FAILED tests/unit/util/test_db.py::test_session_key_to_path - AssertionError: assert None == 'sessions/1'
FAILED tests/unit/util/test_misc.py::test_to_datetime - Failed: DID NOT RAISE <class 'ValueError'>
FAILED tests/unit/util/test_misc.py::test_to_timedelta - Failed: DID NOT RAISE <class 'ValueError'>
FAILED tests/unit/util/test_misc.py::test_add_timezone_info - AttributeError: 'int' object has no attribute 'replace'

@PrestonHager
Copy link
Contributor Author

Yes the four failing tasks, I wrote that way on purpose. The functions they are testing assume no wrong input values which may be a fine precondition and we could remove the tests. The tests are specifically written to raise a ValueError or some other exception when a wrong parameter is entered. For example, entering a value of -1 into the to_datetime function.

Also the test_session_key_to_path needs to be fixed before merging, I couldn't figure out what the function is supposed to return based on the data I had run through the database I had. I ran the ingestor for the year 2023 or 2024 I think.

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.

2 participants