Broken build - Populate campaigns behat failure - graphql4

[emteknetnz]

GraphQL 4 only

https://travis-ci.com/github/silverstripe/silverstripe-campaign-admin/jobs/490659484

tests/behat/features/populate-campaigns.feature:15

PRs

• [x] https://github.com/silverstripe/silverstripe-graphql/pull/380

[unclecheese]

It appears the cause of this issue, and likely https://github.com/silverstripe/silverstripe-asset-admin/issues/1186 as well is an unbuilt graphql artefact. What makes it hard to replicate is that once the error happens once, it never happens again.

GraphQL UIs, particularly asset-admin, reliy on admin.types.graphql being accessible in the public directory. In most cases, you never have to think about this because dev/build builds the schema, and the schema artefact is generated in an event handler that is bound to graphqlSchemaBuild.

We've anticipated that we may need to use UIs before having the schema build, and to mitigate failures, we've placed a fallback in the controller that will generate the schema at request time if absolutely necessary.

But the problem here is that Behat never runs dev/graphql/build, not dev/build, so the event handler is never fired. The database build is handled entirely programatically in TestSessionEnvironment. So we need to fill in the gap. Here are some options:

• Add a new step to the base SilverStripeContext class for I build the admin GraphQL schema
• GraphQL adds an extension to TestSessionEnvironment that invokes the onBeforeStartTestSession hook: https://github.com/silverstripe/silverstripe-testsession/blob/2/src/TestSessionEnvironment.php#L160
• Add a new buildGraphql method to TestSessionEnvironment to make it more of a first-class citizen (would have to be dependent on graphql version)
• Use a generic hook like onAfterApplyState
• Add a "refresh the page" step after navigating to asset-admin (hey, it would work)

[emteknetnz]

Add a new step to the base SilverStripeContext class for I build the admin GraphQL schema

Add a "refresh the page" step after navigating to asset-admin (hey, it would work)

Not too keen on these 2 approaches because they would involve updating existing tests just to work with graphql 4. They're also very easy for devs to forget to do on future tests created, then wonder why their tests are failing for no reason.

Add a new buildGraphql method to TestSessionEnvironment

How would this method get called? Would we need to do something additional on modules like asset-admin?

Use a generic hook like onAfterApplyState

GraphQL adds an extension to TestSessionEnvironment

onAfterApplyState is in the testsession module - so these 2 are functionally the same solution? This seems ideal if I understand it correct - it means we would if Graphql 4 is installed it will automatically do what needs to happen without any other changes required?

[unclecheese]

Yeah that's right. It makes it graphql's problem to make sure the test session is valid. While that's the cleanest approach, I'm not sure it's ideologically correct. If you look through TestSessionEnvironment, it has all kinds of awareness of the CMS, and that's what I meant by adding "buildGraphql" to that layer of concerns (conditional on a version check).

But we're early days, and I think the easiest way to get the builds green ATM is to use one of those extension hooks.

[emteknetnz]

I'm happy to have this implemented via an extension hook if you think that's the best way forward to get the builds green. I'm happy to peer review this for you. Is this something that you're happy to do the development on?