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

Clear up unit tests in redbox-core, add tests for loaders #997

Merged
merged 1 commit into from
Aug 29, 2024

Conversation

wpfl-dbt
Copy link
Collaborator

@wpfl-dbt wpfl-dbt commented Aug 28, 2024

Context

#964 removed async from ingestion. I did a spike on adding it back and decided not to (so this closes #986 ), but in the process wrote unit tests for loaders, and tidied up type hints for fixtures and tests in core.

Changes proposed in this pull request

  • Moves all loaders to a single file (partly because it makes mocking unstructured easier)
  • Adds unit tests for those loaders
  • Adds type hinting to all fixtures and unit tests in Redbox core
  • Standardises client naming conventions across fixtures, aligning with core API

Guidance to review

  • Confirm you're happy with moving the loader classes
  • Confirm my ingest unit tests are alright

Things to check

@wpfl-dbt wpfl-dbt changed the title Refactored unit tests and added type hinting, added ingest unit tests Clear up unit tests in redbox-core, add tests for loaders Aug 28, 2024
@wpfl-dbt wpfl-dbt marked this pull request as ready for review August 28, 2024 17:31
@wpfl-dbt wpfl-dbt mentioned this pull request Aug 28, 2024
3 tasks
@@ -1,6 +1,8 @@
import pytest
from contextlib import nullcontext as does_not_raise
from uuid import uuid4
from pytest_mock import MockerFixture
Copy link
Collaborator

Choose a reason for hiding this comment

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

TIL

Copy link
Collaborator

@gecBurton gecBurton left a comment

Choose a reason for hiding this comment

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

really nicely done. good job on merging the loaders too, thats been annoying me for a while!

@wpfl-dbt wpfl-dbt merged commit fb949db into main Aug 29, 2024
9 checks passed
@wpfl-dbt wpfl-dbt deleted the feature/refactor-redbox-fixtures branch September 26, 2024 09:09
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.

3 participants