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

Improve Testing Infrastructure #87

Merged
merged 23 commits into from
Nov 12, 2024
Merged

Improve Testing Infrastructure #87

merged 23 commits into from
Nov 12, 2024

Conversation

jjnesbitt
Copy link
Member

@jjnesbitt jjnesbitt commented Nov 6, 2024

This PR improves the testing infrastructure of the UVDAT API, through the addition of pytest fixtures, factory-boy factories, and some core tests. These tests aren't fully expansive, and don't cover every possible scenario, but instead attempt to test the core business logic that is already implemented. Many of the default views that we leverage from DRF are not explicitly tested, as we do very little (if any) modification of those viewset behaviors, and as such we would simply be testing DRF itself.

Some side effects of this PR:

  1. Fixed a bug where the dataset list would only show datasets that were used in projects a user was added to.
  2. Fixed a bug that allowed collaborators to change the owner of a project.
  3. The removal of many unused endpoints.

Base automatically changed from remove-derived-regions to master November 7, 2024 16:29
@jjnesbitt jjnesbitt force-pushed the testing-infrastructure branch 3 times, most recently from 19a8959 to e9986a4 Compare November 7, 2024 17:02
@jjnesbitt jjnesbitt marked this pull request as ready for review November 7, 2024 17:02
@jjnesbitt jjnesbitt requested a review from annehaley November 7, 2024 17:13
@jjnesbitt jjnesbitt force-pushed the testing-infrastructure branch from f040029 to d7a31ae Compare November 7, 2024 17:20
@jjnesbitt jjnesbitt force-pushed the testing-infrastructure branch from d7a31ae to a22252a Compare November 7, 2024 17:39
Copy link
Collaborator

@annehaley annehaley left a comment

Choose a reason for hiding this comment

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

I was originally hesitant about removing so much API code, but after some thought I agree that shrinking our API surface to only what we actively use is a good approach for now, especially in the context of fully testing the API. We only need to worrry about a robust API when we get users that would leverage it.

I'm excited about this new test suite, especially since it helped reveal some bugs that you fixed. The permissions testing is much stronger now, and I'm glad for the time we'll get back by scheduling the slow tests.

Just a few questions on minor things, but overall this looks good.

uvdat/core/rest/project.py Show resolved Hide resolved
uvdat/core/tests/conftest.py Show resolved Hide resolved
uvdat/core/tests/test_dataset.py Show resolved Hide resolved
This was linked to issues Nov 12, 2024
@jjnesbitt jjnesbitt merged commit 7f43ba5 into master Nov 12, 2024
4 checks passed
@jjnesbitt jjnesbitt deleted the testing-infrastructure branch November 12, 2024 22:03
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.

Improve test suite Fix flaky pytest tests
2 participants