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

Refactor integration test fixtures #456

Merged
merged 1 commit into from
Jul 18, 2023

Conversation

nenb
Copy link
Contributor

@nenb nenb commented Jul 17, 2023

Description

This PR refactors the test fixtures. It creates separate conftest.py files for the integration and unittests,
and then simplifies the fixtures in the conftest.py file for the integration tests. This was necessary as
there were so many layered fixtures that it was hard to tell what the initial state was before any test ran.

The refactor of the test fixtures also led to some changes in the original tests. I do not believe that I have
altered them in any substantial way, but they should be checked just in case.

I have also refactored the server API slightly, to make it more amenable to testing. I don't believe that this has any major impact on the code.

A later PR will do something similar for the unittests.

Related Issue(s)

#388

Checklist

  • Change or addition is covered by unit and/or integration tests. If bugfix, there should be at least one test that fails pre-PR and passes after.
  • Classes and functions substantially affected by this PR have sphinx format docstrings added or updated.
  • If your changes introduce modifications to functionality, behavior, or usage of the project, please ensure that the documentation is updated accordingly.

Additional Notes

The project really needs to figure out what it means by integration and unittests! A bunch of the current 'unittests' take longer than the integration tests, and have at least as much coupling to a live database service.

@nenb nenb force-pushed the refactor-test-fixtures branch from ca52bb5 to 849c93f Compare July 17, 2023 18:22
@nenb nenb force-pushed the refactor-test-fixtures branch 2 times, most recently from ea8cf76 to f1cebe3 Compare July 17, 2023 18:56
@nenb nenb requested review from blythed, thejumpman2323 and rec and removed request for blythed July 17, 2023 19:03
@nenb nenb marked this pull request as ready for review July 17, 2023 19:04
Copy link
Contributor

@thejumpman2323 thejumpman2323 left a comment

Choose a reason for hiding this comment

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

Great Work! some nitpicks I have added

superduperdb/serve/server.py Show resolved Hide resolved
tests/integration/conftest.py Show resolved Hide resolved
tests/integration/test_cdc.py Outdated Show resolved Hide resolved
tests/integration/test_cdc.py Show resolved Hide resolved
tests/integration/test_dask.py Show resolved Hide resolved
tests/unittests/conftest.py Show resolved Hide resolved
@nenb nenb force-pushed the refactor-test-fixtures branch from f1cebe3 to 50f1de7 Compare July 18, 2023 07:20
Copy link
Collaborator

@blythed blythed left a comment

Choose a reason for hiding this comment

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

Great, let's get this merged.

Copy link
Contributor

@rec rec left a comment

Choose a reason for hiding this comment

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

So I didn't lose interest in the second half :-P, I just didn't see anything to suggest.

tests/integration/conftest.py Show resolved Hide resolved
tests/integration/conftest.py Outdated Show resolved Hide resolved
tests/integration/conftest.py Outdated Show resolved Hide resolved
tests/integration/conftest.py Outdated Show resolved Hide resolved
tests/integration/conftest.py Outdated Show resolved Hide resolved
tests/integration/conftest.py Show resolved Hide resolved
tests/integration/conftest.py Outdated Show resolved Hide resolved
tests/integration/conftest.py Show resolved Hide resolved
tests/integration/conftest.py Show resolved Hide resolved
tests/integration/test_cdc.py Outdated Show resolved Hide resolved
@nenb nenb force-pushed the refactor-test-fixtures branch from 50f1de7 to 098c5d6 Compare July 18, 2023 15:50
@nenb nenb merged commit 9661ff8 into superduper-io:main Jul 18, 2023
@nenb nenb deleted the refactor-test-fixtures branch July 18, 2023 15:57
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.

4 participants