reworked import test+ worked on fresh branch #246
Conversation
|
This took longer than anticipated. |
| def test_ContentPageIndex_required_fields(self, csv_impexp: ImportExport) -> None: | ||
| """ | ||
| Importing an CSV file with only the required fields shoud not break | ||
|
|
There was a problem hiding this comment.
No need for these empty lines at the end of the docstrings.
| content = csv_impexp.export_content() | ||
| src, dst = csv_impexp.csvs2dicts(csv_bytes, content) | ||
|
|
||
| content_pages = ContentPageIndex.objects.all() |
There was a problem hiding this comment.
Since we know what the source data is, we can be a lot more explicit with the assertions:
[main_menu] = ContentPageIndex.objects.all()
assert main_menu.slug == "main-menu"The first line will fail if we don't have exactly one ContentPageIndex, and the second will fail if the slug doesn't match.
There was a problem hiding this comment.
I kept this line since I use that to check how many pages/ indexes we have, but I could find a work around to get that data directly from csv_bytes, not sure.
There was a problem hiding this comment.
Comparing the original and re-exported CSVs will ensure that the line/page counts are the same, and the [main_menu] = ... assignment will raise an exception if the sequence on the right doesn't have exactly one element. 🙂
There was a problem hiding this comment.
I see, I brought back the part that compares the imported and exported files. I had to clean it up the exported file a bit because the exporter adds extra fields we don't want to check for.
|
|
||
| content_pages = list(ContentPageIndex.objects.all()) + list( | ||
| ContentPage.objects.all() | ||
| ) |
There was a problem hiding this comment.
As with the previous test, we can be a lot more explicit:
[main_menu] = ContentPageIndex.objects.all()
[first_time_user, health_info, self_help] = ContentPage.objects.all()
# And then a slug assertion for each page.There was a problem hiding this comment.
Got it, I kept the line that defines content_pages so checking the sum a a bit cleaner.
There was a problem hiding this comment.
Do we really need to assert on the sum if we already get an exception when any of the queries returns the wrong number of results?
There was a problem hiding this comment.
Correct, no need for that check, removed.
| content = csv_impexp.export_content() | ||
| src, dst = csv_impexp.csvs2dicts(csv_bytes, content) | ||
|
|
||
| # the importer adds extra fields, so we fikter for the ones we want |
There was a problem hiding this comment.
Tyop in this comment (and the one below): fikter -> filter.
Fixed the import test. Check the db state as opposed to comparing the input and output files directly. Also switched to new branch since the old one got too messy.