Skip to content
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

EZEE-2718: Adjust tests for redirects #900

Merged
merged 1 commit into from
Mar 15, 2019
Merged

EZEE-2718: Adjust tests for redirects #900

merged 1 commit into from
Mar 15, 2019

Conversation

mnocon
Copy link
Member

@mnocon mnocon commented Mar 14, 2019

After https://jira.ez.no/browse/EZEE-2718 the behaviour after Trashing a content item has changed:
Before:
Trashing a Content Item that has a Landing Page as a parent redirects to the Parent in Content View
After:
Trashing a Content Item that has a Landing Page as a parent redirects to the Parent:

  • ezplatform: in Content View
  • ezplatform-ee: in Page Builder

This is a bit tricky, as we don't have a mechanism right now to deal with logic like this.

Possible solutions:

  1. the proposed one - adding an IF based on environment

I'm not a fan of this, but it works - we need to think of a better approach for future uses.

  1. Defining the same step definition in two Contexts and specifying the correct one in behat_suites.yml

I've decided against at as it does not work for use cases like "I want to run AdminUI suite on ezplatform-ee (we would have to define the suite twice, with the different Context being the only difference)

  1. Creating two scenarios, tagging it with ezplatform and ezplatform-ee and filtering it in suites:

Again, this does not work for use case "I want to run AdminUI suite on ezplatform-ee"

  1. Creating two scenarios, tagging it with ezplatform and ezplatform-ee and filtering in an BeforeScenario hook

This works, but only partially - the ezplatform-ee scenario might require steps that are not defined in any AdminUI Context, so it will fail when running with --strict flag because of undefined steps

  1. Create a Behat extension that will allow to skip tests marked with a certain tag (for example "ezplatform" when running on "ezplatform-ee") and will not mark undefined steps in these Scenarios as missing

I've an idea how to do it, but it's not something I can do right now.

@ezsystems ezsystems deleted a comment from ezrobot Mar 15, 2019
@@ -15,13 +15,6 @@ class Hooks extends RawMinkContext
{
use KernelDictionary;

/** @BeforeScenario
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've decided to use this opportunity to remove this, I don't think we need it and it causes failures from time to time.

@ezsystems ezsystems deleted a comment from ezrobot Mar 15, 2019
@ezsystems ezsystems deleted a comment from ezrobot Mar 15, 2019
@ezsystems ezsystems deleted a comment from ezrobot Mar 15, 2019
@mnocon mnocon changed the title [WIP] [DON'T MERGE] EZEE-2718: Adjust tests for redirects [WIP] EZEE-2718: Adjust tests for redirects Mar 15, 2019
@mnocon mnocon changed the title [WIP] EZEE-2718: Adjust tests for redirects EZEE-2718: Adjust tests for redirects Mar 15, 2019
@mnocon mnocon requested review from m-tyrala and micszo March 15, 2019 13:52
Copy link
Contributor

@m-tyrala m-tyrala left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not entirely sure what do you mean by 5. option (so I'll be happy to talk about it when you'll work on it), but fix itself is fine for me.

Copy link
Member

@micszo micszo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍 to unblock tests.

@lserwatka
Copy link
Member

@mnocon is this ready for merge?

@mnocon
Copy link
Member Author

mnocon commented Mar 15, 2019

@lserwatka yes, this one can be merged

@lserwatka lserwatka merged commit 308701e into ezsystems:1.4 Mar 15, 2019
@lserwatka
Copy link
Member

Great work! Could you merge it up?

@mnocon
Copy link
Member Author

mnocon commented Mar 15, 2019

Done

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

4 participants