Some of the tests depend on panopoly_test, and this is only going to become more and more the case. We should make sure panopoly_test is enabled before running.

This way we don't have to document that you need to enable panopoly_test before running the tests, which could be confusing because it's 'hidden' and won't appear in any of the UIs.

CommentFileSizeAuthor
#4 panopoly_test-ensure-module-2296573-4.patch839 bytesdsnopek
#2 panopoly-enable-disable-test-2296573-1.patch1.44 KBdsnopek
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

dsnopek’s picture

dsnopek
he/him
Primary language English
Location USA
CreditAttribution: dsnopek commented

Behat's API actually makes this quite difficult for us! I've got an attempt which I want to test on Travis-CI before committing:

https://travis-ci.org/dsnopek/panopoly/builds/28975693

dsnopek’s picture

dsnopek
he/him
Primary language English
Location USA
CreditAttribution: dsnopek commented
Status: Active » Closed (won't fix)
FileSize
panopoly-enable-disable-test-2296573-1.patch1.44 KB

The tests appear to be passing on Travis-CI. However, given how weird this is to do and the fact that it slows the tests down (which are already pretty slow), I think I'm going to back down on this in Panopoly. I'm attaching the patch in case anyone wants to implement this in their child distribution.

dsnopek’s picture

dsnopek
he/him
Primary language English
Location USA
CreditAttribution: dsnopek commented
Title: panopoly_test should be enabled/disabled automatically by Behat » Behat tests should error out if panopoly_test isn't enabled
Issue summary: View changes
Status: Closed (won't fix) » Needs work

Re-worked this issue to be about erroring out if panopoly_test isn't enabled, rather than trying to enable/disable it automatically.

dsnopek’s picture

dsnopek
he/him
Primary language English
Location USA
CreditAttribution: dsnopek commented
Title: Behat tests should error out if panopoly_test isn't enabled » Behat tests should error if panopoly_test isn't enabled
Status: Needs work » Needs review
FileSize
panopoly_test-ensure-module-2296573-4.patch839 bytes

This turned out to be a little harder than I had hoped.

First of all, we can't put this in panopoly_test.behat.inc, because if panopoly_test is disabled, that file won't be loaded. This must be a new Behat 3 thing, because in the past I'm pretty sure it always loaded it.

Second, we can't do this on @BeforeSuite because the Drupal driver isn't initialized at that point, so we have no way of listing the enabled modules. So, we have to do it on @BeforeScenario, which is fine because listing the modules is a super fast operation.

However, I'm unable to find a way to abort the test entirely in an orderly way. I suppose we could do exit(1) but that's pretty nastly. I was hoping for an exception that would abort the whole thing and still allow normal Behat output.

So, the net result is that it runs through each of the scenarios, marking them as failed with a message about needing to enable 'panopoly_test'.

This is probably better than failing without a helpful message! But I'll need to think on this to see if there is a better way...

cboyden’s picture

cboyden CreditAttribution: cboyden commented

I think the approach of throwing an exception in the BeforeScenario is fine, it'll just zoom through all the scenarios failing each one. (But there is always the kludge of getting at the drush parameters and calling drush directly BeforeSuite.) I'm actually more concerned about putting this into FeatureContext. Maybe that's irrational in the new world of Behat 3 where FeatureContext is no longer magical? Anyway, maybe instead we should make a new SetupContext file that has this BeforeSomething function, then add it to behat.common.yml? Or, even better, add it to behat.template.yml, so anyone who wanted to leave it out could do so in their customized behat.yml.

That would also be a bit of a bonus in that it would quite visibly demonstrate that you can add your own contexts in your behat.yml.

dsnopek’s picture

dsnopek
he/him
Primary language English
Location USA
CreditAttribution: dsnopek commented

Well, since it's in FeatureContext, it means that child distributions don't automatically get it, which is the main downside of putting it in there. It also kinda sucks because some of the steps in panopoly_test.behat.inc depend on the panopoly_test module, so there is definitely a symmetry to having the check be in that sub-context rather than FeatureContext.

I think having a SetupContext has the same weaknesses as using FeatureContext: child distributions would still need to copy it into their test suite to use it...

One interesting thing about the way Behat 3 doesn't load panopoly_test.behat.inc when panopoly_test isn't loaded, is that it'll now say that all of the steps from there are undefined, which is a little more of a clue than just having the tests fail.

cboyden’s picture

cboyden CreditAttribution: cboyden commented

Would it be crazy to put a .behat.inc that just had this function into panopoly_core? Then you'd get the behavior if core were enabled.

dsnopek’s picture

dsnopek
he/him
Primary language English
Location USA
CreditAttribution: dsnopek commented

But what if you used panopoly_core in your child distribution, but didn't use panopoly_test and just wrote your own Behat tests? Then it'd complain about 'panopoly_test' not being enabled, but you don't actually need it for anything...

cboyden’s picture

cboyden CreditAttribution: cboyden commented

You're right about panopoly_core.

If this is worth doing, I think the best way to do it is in a Context file that we put into behat.template.yml (not behat.common.yml) with some inline comments that will point people in the right direction.