-
Notifications
You must be signed in to change notification settings - Fork 4
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(excel2json): modularise unittests (DEV-3040) #659
Conversation
DEV-3040 modularise test
Currently many different function calls for testing are combined in one function. If a test fails it is harder to understand which test failed. |
Before: The function created the resource section and in the class set up saved it as a json in a temp folder. It then read the json and checked that it was the same as the input from the method. Saving the json is not something we need to test. But it significantly complicated this test. Instead now we just take the output from the function and extract the information. No temp needed.
Here again we created a complex test, for a file that was written, with this changed function call we can test the results without having to read and save a temp file.
See: 2d3cfa6 for my reasoning |
As discussed with Balduin (#654 (comment)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This PR produced a pretty gnarly diff, so I did more of a skimming than a proper review - but I didn't see any show stoppers, and I like the general notion of the PR, so I would just trust you on this one
Thanks. The only conceptual difference I made is that I got rid of this temp file writing and reading. So if that makes sense to you, the rest is just more pytest and breaking up big functions, but no content change. |
It absolutely does. |
That is a mish-mash I am not fond of myself. But The reason why it isn't, is because I could not find a simple and easy pytest equivalence for dictEqual at all, so I would have to compare keys and then iterate etc. So I kept the unittest. This is the same in the other files. |
No description provided.