-
Notifications
You must be signed in to change notification settings - Fork 4
Double down on human readable fixtures and support via extended hints. #349
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
base: next
Are you sure you want to change the base?
Conversation
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.
On a quick glance the overall idea looks good and useful 👍
I'll comment a bit inline with a consideration or suggestion for the practical handling of the offset dates.
"expire": { | ||
"_FixtureHint": true, | ||
"hint": "time_from_current", | ||
"modifier": "days|+5", |
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.
I wonder if it's a great idea to have the dynamics and logic embedded directly in the json 🤔 or if it would in fact be better to simply have a dumb replacement marker (e.g. __EXPIRE__
) there and require it to always be actively expanded by the unit tests when using the fixture.
Alternatively going all-in and (re)use e.g. jinja2 templating and have these known dynamic values filled by the engine.
The main point would be to actually keep all logic contained in the tests rather than spreading it across fixtures and tests.
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.
I thought about this too but concluded differently. There is still a distinct separation here: the fixture data specifies exactly what ought to be the case when it is loaded, and tests only see the resultant structure. In addition, I need this in the data itself rather than in the test so that it can be used for e.g. writing a test user database.
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.
As of the latest push you can see the effect of this here:
https://github.com/ucphhpc/migrid-sync/pull/349/files#diff-fed3248b2c06db6ed9ddce22baba29d9a62e32d47cb2b596e19d4b1d867d97c1R376
The JSON on-disk fixture is used as the basis of the pickled user database consumed by other things. This actually ends up working really well because if any changes to that basic user are required, you simply edit the fixture.
94d25d6
to
e4ebf24
Compare
e4ebf24
to
f0dc727
Compare
f0dc727
to
476fca6
Compare
2e61bd4
to
e0cb9f5
Compare
One of the key aspects of using fixtures is maintaining the ability to know their contents with ease. This was slightly undermined by allowing pickled fixture data. Use the opportunity presented by having to handle the inclusion of timestamps at now points in the future, something that is something of a necessity for use of fixtures going forward, both to add support for doing so but push it further so such values are specified within a file readable format. There were pre-existing provisions for loading JSON as well as being able to specify additional transformations of properties to perform as a series of hints. Extend and make use of this mechanism for a new fixture output path - this allows fixture data to be written out in a forat different than it is read in (which can include transformation). Make use of this new support to rework the test user provisioning. The use is now defined in JSON and, rather than copy a pre-existing file representing the requisite user database, write this database based on the JSON data. Do the work necessary to make sure the data in this file is output with strings transformed into bytes as is the format of the user database.
e0cb9f5
to
f9fab4c
Compare
One of the key aspects of using fixtures is maintaining the ability to
know their contents with ease. This was slightly undermined by allowing
pickled fixture data.
Use the opportunity presented by having to handle the inclusion of
timestamps at known points in the future, something that will only be
more of a necessity with fixture use going forward, both to add support
for doing so but push it further so such values are specified within
the readable format itself. The effect is quite nice: at the point of
being loaded a fixture already contains the requested data, so not
only can change the value easily but we maintain firm encapsulation.
There were pre-existing provisions for loading JSON as well as being
able to specify additional transformations of properties to perform
as a series of hints. Extend and make use of this mechanism for a new
fixture output path - this allows fixture data to be written out in a
forat different than it is read in (which can include transformation).
Make use of this new support to rework the test user provisioning. The
use is now defined in JSON and, rather than copy a pre-existing file
representing the requisite user database, write this database based
on the JSON data. Do the work necessary to make sure the data in this
file is output with strings transformed into bytes as is the format
of the user database.