Page MenuHomePhabricator

Restructure dagster-graphql tests to control context lifecycle

Authored by schrockn on Thu, May 14, 9:40 PM.



With the dagster-graphql becoming a proper "host" process, we
will be launching lots of user processes from test cases. As a result
we need to be more disciplined about managing and cleaning up resources.

This was the genesis for this diff, as this test libraries predilection
for newing up contexts whenever and whereever it pleased made the managing
that untractable.

This adds a fixture for the graphql context and then uses it in the vast
majority of tests. This can be a common codepath for doing cleanup and
management (in this case of temp files, but we are going to have clean
up processes).

This also opens us up doing something like running a suite of tests against
other common configurations (e.g. with postgres) by changing what context
we new up against these.

For cases where we don't use the graphql_context fixture, I eliminated all
optional instance parameters in context creation functions. This forces
the tests to manage the lifecycle of the instances is they want to roll their own.

Test Plan


Diff Detail

R1 dagster
Automatic diff as part of commit; lint not applicable.
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

schrockn created this revision.Thu, May 14, 9:40 PM
schrockn edited the summary of this revision. (Show Details)Thu, May 14, 9:58 PM
schrockn requested review of this revision.Thu, May 14, 10:13 PM
max added a comment.Thu, May 14, 10:18 PM

sweet, this is much better

max accepted this revision.Thu, May 14, 10:18 PM
This revision is now accepted and ready to land.Thu, May 14, 10:18 PM