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

Cleanup: remove test_settings_module #663

Merged
merged 3 commits into from Oct 21, 2022

Conversation

jayaddison
Copy link
Collaborator

Removes the test_settings_module, which is/was used mainly to enable suppression of exceptions during unit tests.

This uncovers a few exceptions that were occurring during the tests: mostly for cases where we had test coverage on fields for data that wasn't confirmed to be present on each page. Those are mostly removed.

@jayaddison
Copy link
Collaborator Author

Sorta makes me wonder whether removing ratings from the scraper template could make sense, if it's frequently unavailable. Or perhaps a comment to indicate that it's fine to remove/omit it that method's implementation when information is not present.

@bickerdyke
Copy link
Contributor

A recipe does not necessarily need to be rated, so it shouldn't be a required field in a way that raises an exception when not available.

@jayaddison
Copy link
Collaborator Author

@bickerdyke hypothetical example: if recipe site http://example.org has recipes without ratings information, and we have a scraper called ExampleScaper to handle that site, what should calling .ratings() do for that scraper?

(currently it does raise an exception - not because the field is required, but because there's no implementation for it)

@jayaddison jayaddison merged commit d839b76 into main Oct 21, 2022
@jayaddison jayaddison deleted the issue-617/cleanup-remove-test-settings-module branch October 21, 2022 17:16
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.

None yet

2 participants