Repair the Selenium Behat tests #291
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
MKodde
force-pushed
the
maintenance/behat-repairs
branch
from
5c4b610
to
063de42
Compare
johanib
approved these changes
Aug 14, 2023
public/index.php
Outdated
| // When running the selenium tests, a test cookie indicate if we should switch to the smoketest env | ||
| $hasTestCookie = (array_key_exists('testcookie', $_COOKIE) && $_COOKIE['testcookie'] === 'testcookie'); | ||
|
|
||
| if (($isTestOrDev && $_SERVER['HTTP_USER_AGENT'] === 'Symfony BrowserKit') || ($isTestOrDev && $hasTestCookie)) { |
There was a problem hiding this comment.
Suggested change
| if (($isTestOrDev && $_SERVER['HTTP_USER_AGENT'] === 'Symfony BrowserKit') || ($isTestOrDev && $hasTestCookie)) { | |
| if ($isTestOrDev && ($_SERVER['HTTP_USER_AGENT'] === 'Symfony BrowserKit' || $hasTestCookie)) { |
There was a problem hiding this comment.
Yes, that is more betterest, going to fix that up!
| @@ -164,6 +168,8 @@ public function iOpenTwoBrowserTabsIdentifiedBy($numberOfTabs, $tabNames) | |||
| 'Please identify all tabs you are opening in order to refer to them at a later stage' | |||
| ); | |||
| } | |||
| // Set the testcookie ensuring the selenium tests run in the smoketest env | |||
| $this->getMink()->getSession()->setCookie('testcookie', 'testcookie'); | |||
There was a problem hiding this comment.
is it possible to use more descriptive naming? For example: 'isBehatTest'
There was a problem hiding this comment.
A good suggestion, but I choose to adhere to the way we called this cookie in OpenConext-engineblock. That way, we have feature parity between the projects.
Test one and two have been fixed. The Yubikey and SMS tests could be enabled in the new (SSP based) test environment. The Tiqr variant could not yet be enabled as Tiqr is not functioning in that environment yet. During enabling the SMS test, I found out that the SMS Bypass fix was not taken into account in the existing sfo test. That has been updated.
Previously the page_title block was rendered, but that was not set. The application_name was set with the 'gateway.second_factor.choose_second_factor.title' and using that on the page title made perfect sense to me.
MKodde
force-pushed
the
maintenance/behat-repairs
branch
from
063de42
to
2ed27e9
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The SFO multiple authentications feature was disabled a while ago. They have been re-instated in this PR
Test one and two have been fixed. The Yubikey and SMS tests could be enabled in the new (SSP based) test environment. The Tiqr variant could not yet be enabled as Tiqr is not functioning in that environment yet.
During enabling the SMS test, I found out that the SMS Bypass fix was not taken into account in the existing sfo test. That has been updated.
Commit 2 consists of a boyscout rule fix.
@pmeulen and or @phavekes
We might not want to change this behavior for the 4.2 release?