-
Notifications
You must be signed in to change notification settings - Fork 28
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
Add Behat feature and (context) to view a list of versions in History Viewer #14
Add Behat feature and (context) to view a list of versions in History Viewer #14
Conversation
.travis.yml
Outdated
@@ -38,7 +38,7 @@ before_script: | |||
- export PATH=~/.composer/vendor/bin:$PATH | |||
- echo 'memory_limit = 2048M' >> ~/.phpenv/versions/$(phpenv version-name)/etc/conf.d/travis.ini | |||
|
|||
# Install composer | |||
# Install composer |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Might be worth resetting this, it doesn't add value
/** | ||
* @Then I should see a list of versions in descending order | ||
*/ | ||
public function iShouldSeeAListOfVersionsInDescendingOrder() { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
PSR-2 (opening brace on new line)
$this->assertElementContains('.history-viewer .badge', $arg1); | ||
} | ||
|
||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you add PHPdocs to these protected methods please, to explain what they do?
$versionColumn = $version->find('css', 'td'); | ||
|
||
$exists = strpos($versionColumn->getText(), $text) !== false; | ||
assertTrue($exists, 'Version column contains ' . $text); } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fix up the brace at the EOL
Then I should see a list of versions | ||
And I should see "ADMIN User" in the author column in version 1 | ||
And I should see "Saved" in the record column in version 1 | ||
And I should see "01/01/2100" in the record column in version 1 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe we should add And I should not see the "Live" badge
to ensure it doesn't show up when nothing is published yet? We can modify the regex for the And I should see the :arg badge
step definition to support an optional not
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I like the idea, but I am having trouble getting it to work at the moment. Below my approach:
/**
* Example: I should see the "Live" badge
* Example: I should not see the "Live" badge
*
* @Then /^I should( not? |\s*)see the :text badge
* @param string $negative
* @param string $text
*/
public function iShouldSeeTheBadge($negative, $text)
{
if (trim($negative)) {
$this->assertElementContains('.history-viewer .badge', $text);
} else {
$this->assertElementNotContains('.history-viewer .badge', $text);
}
}
How important do you reckon this is at this stage?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think you need to name the regex capture group for it to be assigned to a variable, e.g.:
@Then /^I should (?P<negative>(?:(not |)))see the :text badge
...
public function iShouldSeeTheBadge($negative, $text)
{
As a cms author | ||
I want to preview a selected version | ||
|
||
// Given a preview panel exists for this page |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This feature looks incomplete - remove?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great that you caught that. It is WIP indeed and I only accidentally added it to the PR.
Then I should see an "#Form_versionForm_Title[readonly]" element | ||
And I should see an "#Form_versionForm_URLSegment[readonly]" element | ||
Then I should see an "#Form_versionForm_URLSegment[readonly]" element |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think And
was better actually
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should be available in core - I've logged it here: silverstripe/silverstripe-behat-extension#177
4f5122b
to
879fdae
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks great. Merge when the CI builds are green =)
879fdae
to
cbdaced
Compare
builds are green @robbieaverill - I'm not merging because I'm not really on top of this module |
Additions were made to the FeatureContext to allow to assert text being contained within specific columns within specific versions. The intention is that this allows modularity for further tests.
Note that the following workaround
And I go to "/admin/pages/history/show/1"
was used to navigate to the History, rather than clicking the History tab. This is because of the popup notice that hides (and prevents from clicking) the History tab - see: silverstripe/silverstripe-cms#2128 Unfortunately, dismissing the pop up and waiting for it to fade away has been unreliable.