We need to ensure that tips corresponding page elements to not get removed (or need to be adjusted).

We are to provide a TourTestBase class that will aid in the creation of tests for this.

Basic usage:

<?php
 
function testTourViewEditBasic() {
   
$this->drupalLogin($this->admin);
   
$this->drupalGet('foo');
   
$this->assertTourTips();
  }
}
?>

Advanced usage (in the case of multipage or targetting a subset of tips):

<?php
 
function testTourViewEditBasic() {
   
$this->drupalLogin($this->admin);
   
$this->drupalGet('foo');
   
$tips = array();
   
$tips[] = array('data-id' => 'foo');
   
$tips[] = array('data-id' => 'bar');
   
$tips[] = array('data-class' => 'baz');
   
$this->assertTourTips($tips);
  }
}
?>

We also provide a basic class that can be implemented to ensure all tips are available on a page (this is ideal for very simple one page tours):

<?php
 
/**
 * @file
 * Contains \Drupal\system\Tests\SystemTour.
 */
 
namespace Drupal\system\Tests;
 
use
Drupal\tour\Tests\TourTestBasic;
 
/**
 * Tests tour functionality.
 */
class SystemTour extends TourTestBasic {
 
  protected
$tips = array('/admin/modules' => array());
 
  protected
$permissions = array('administer modules', 'access tour');
 
  public static
$modules = array('tour');
 
  public static function
getInfo() {
    return array(
     
'name' => 'Extend tour tests',
     
'description' => 'Tests the Extend tour.',
     
'group' => 'Tour',
    );
  }
}
?>
Files: 
CommentFileSizeAuthor
#59 2028535-59-tour-test-unused.patch847 byteststoeckler
PASSED: [[SimpleTest]]: [MySQL] 60,272 pass(es).
[ View ]
#56 interdiff.txt2.96 KBnick_schuch
#56 2028535-55-tourtestbase.patch9.79 KBnick_schuch
PASSED: [[SimpleTest]]: [MySQL] 57,931 pass(es).
[ View ]
#54 interdiff.txt1.63 KBnick_schuch
#54 2028535-54-tourtestbase.patch7.26 KBnick_schuch
PASSED: [[SimpleTest]]: [MySQL] 57,587 pass(es).
[ View ]
#42 interdiff.txt1.44 KBnick_schuch
#42 2028535-42-tourtestbase.patch6.85 KBnick_schuch
PASSED: [[SimpleTest]]: [MySQL] 57,807 pass(es).
[ View ]
#40 interdiff.txt3.05 KBnick_schuch
#40 2028535-40-tourtestbase.patch6.85 KBnick_schuch
PASSED: [[SimpleTest]]: [MySQL] 57,302 pass(es).
[ View ]
#37 2028535-37-tourtestbase.interdiff.txt1.66 KBnick_schuch
#37 2028535-37-tourtestbase.patch8.45 KBnick_schuch
PASSED: [[SimpleTest]]: [MySQL] 57,472 pass(es).
[ View ]
#32 2028535-32-tourtestbase.interdiff.txt624 bytesnick_schuch
#32 2028535-32-tourtestbase.patch8.35 KBnick_schuch
PASSED: [[SimpleTest]]: [MySQL] 57,409 pass(es).
[ View ]
#30 2028535-30-tourtestbase.interdiff.txt2.32 KBnick_schuch
#30 2028535-30-tourtestbase.patch8.37 KBnick_schuch
PASSED: [[SimpleTest]]: [MySQL] 57,134 pass(es).
[ View ]
#28 2028535-28-tourtestbase.interdiff.txt3.68 KBnick_schuch
#28 2028535-28-tourtestbase.patch8.42 KBnick_schuch
PASSED: [[SimpleTest]]: [MySQL] 57,540 pass(es).
[ View ]
#25 2028535-25-tourtestbase-FAIL.patch7.16 KBnick_schuch
FAILED: [[SimpleTest]]: [MySQL] 57,108 pass(es), 1 fail(s), and 0 exception(s).
[ View ]
#25 2028535-25-tourtestbase-PASS.patch6.69 KBnick_schuch
PASSED: [[SimpleTest]]: [MySQL] 57,522 pass(es).
[ View ]
#22 2028535-22-tourtestbase.interdiff.txt12.93 KBnick_schuch
#22 2028535-22-tourtestbase-FAIL.patch15.29 KBnick_schuch
PASSED: [[SimpleTest]]: [MySQL] 57,030 pass(es).
[ View ]
#22 2028535-22-tourtestbase-PASS.patch15.76 KBnick_schuch
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch 2028535-22-tourtestbase-PASS.patch. Unable to apply patch. See the log in the details link for more information.
[ View ]
#20 2028535-20-tourtestbase.patch5.26 KBnick_schuch
PASSED: [[SimpleTest]]: [MySQL] 56,754 pass(es).
[ View ]
#13 tour-test-base-2028535-13.patch20.34 KBclemens.tolboom
PASSED: [[SimpleTest]]: [MySQL] 56,362 pass(es).
[ View ]
#12 tour-test-base-2028535-12.patch20.4 KBclemens.tolboom
PASSED: [[SimpleTest]]: [MySQL] 58,976 pass(es).
[ View ]
#12 tour-test-base-2028535-12-do-not-test.patch12.16 KBclemens.tolboom
#9 tour-test-base-2028535-9.patch249.66 KBclemens.tolboom
FAILED: [[SimpleTest]]: [MySQL] 58,214 pass(es), 1 fail(s), and 0 exception(s).
[ View ]
#9 interdiff-2028535-5-9.txt2.88 KBclemens.tolboom
#5 interdiff-2028535-3-5.txt3.17 KBclemens.tolboom
#5 tour-test-base-2028535-5.patch8.61 KBclemens.tolboom
FAILED: [[SimpleTest]]: [MySQL] 58,005 pass(es), 15 fail(s), and 0 exception(s).
[ View ]
#3 tour-test-base-2028535-3.patch7.05 KBclemens.tolboom
PASSED: [[SimpleTest]]: [MySQL] 57,979 pass(es).
[ View ]

Comments

clemens.tolboom’s picture

We should validate tours for their schema as noted in #2027469: View UI Tour is not working and had a sandbox update.

clemens.tolboom’s picture

Assigned:Unassigned» clemens.tolboom

I'm working on this.

clemens.tolboom’s picture

Status:Active» Needs review
StatusFileSize
new7.05 KB
PASSED: [[SimpleTest]]: [MySQL] 57,979 pass(es).
[ View ]

Attached patch creates a base test for views_ui tour

It needs more testing on the currently available tours in the issue queue as some tours have module dependent tips

clemens.tolboom’s picture

Status:Needs review» Needs work
+++ b/core/modules/tour/lib/Drupal/tour/Tests/TourTestBase.php
@@ -0,0 +1,133 @@
+  function tipOnPage($tip, $selector) {
+    //TODO:
+    // Use the #tour selector
+    $path = '//li[';
+    if (isset($selector[':data-id'])) {
+      $path .= '@data-id=:data-id and ';
+    }
+    if (isset($selector[':data-class'])) {
+      $path .= '@data-class=:data-class and ';
+    }
+    $path .= ' ./h2]';
+    $elements = $this->xpath($path, $selector);
+    $tip_id = $tip['id'];
+    $this->assertEqual(count($elements), 1, format_string("Found tip for path '$path' on '$tip_id' ", $selector));
+  }
+

This should not test for the aria values but the real page elements.

clemens.tolboom’s picture

StatusFileSize
new8.61 KB
FAILED: [[SimpleTest]]: [MySQL] 58,005 pass(es), 15 fail(s), and 0 exception(s).
[ View ]
new3.17 KB

Attached patch add testing on tip targets pointed to by data-id and data-class but fails on data-class as those can sub selects.

Ie core/modules/views_ui/config/tour.tour.views-ui.yml has

data-class: views-display-top li.active
data-class: views-ui-display-tab-bucket.fields
data-class: views-ui-display-tab-bucket.filter-criteria
data-class: views-ui-display-tab-bucket.filter-criteria .dropbutton-widget
data-class: views-ui-display-tab-bucket.format
data-class: views-ui-display-tab-bucket.sort-criteria
data-class: views-ui-display-tab-bucket.sort-criteria .dropbutton-widget

For xpath to work that should be converted into something like //p[contains(concat(" ",@class," "),concat(" ","sb-fs"," "))][contains(concat(" ",@class," "),concat(" ","bar"," "))] taken from example on http://cowburn.info/2009/06/15/xpath-css-class/

We should use http://symfony.com/doc/current/components/css_selector.html for this I guess :-/

clemens.tolboom’s picture

Status:Needs work» Needs review

Let's run the tests so it's failure become logged :-/

clemens.tolboom’s picture

+++ b/core/modules/tour/lib/Drupal/tour/Tests/TourTestBase.php
@@ -126,8 +133,44 @@ function tipOnPage($tip, $selector) {
+  function tipTargetOnPage($tip_id, $selector) {
+    $parts = array();
+    if (isset($selector[':data-id'])) {
+      $parts[] = '@id=:data-id';
+    }
+    if (isset($selector[':data-class'])) {
+      // We need to convert css selector into xpath
+      //
+      // .views-ui-display-tab-bucket.filter-criteria .dropbutton-widget > .blink
+      //
+      // This needs more work as class can contain multiple classes and sub selects
+      // expand . to 'and' and sub selects
+      // > . => /
+      //  . => //
+      // @see https://drupal.org/project/emogrifier
+      // @see https://github.com/symfony/CssSelector
+      $path = trim($selector[':data-class']);
+      //$path = CssSelector::toXPath('.' . $selector[':data-class']);
+      //$this->verbose($path);
+      //$parts[] = $path;
+      $parts[] = 'contains(@class, :data-class)';
+    }
+    // Target can be anything / anywhere.
+    $path = '//*[' . join(" and ", $parts) . ']';
+    $elements = $this->xpath($path, $selector);
+    $this->assertEqual(count($elements), 1, format_string("Found target for selection '$path' on '$tip_id' ", $selector));

This needs a rewrite on the class selectors.

1. Is it possible at all having both a data-id and data-class?

2. How to get a proper XPath expression on the mentioned 'long' data-class values?

Status:Needs review» Needs work

The last submitted patch, tour-test-base-2028535-5.patch, failed testing.

clemens.tolboom’s picture

Status:Needs work» Needs review
StatusFileSize
new2.88 KB
new249.66 KB
FAILED: [[SimpleTest]]: [MySQL] 58,214 pass(es), 1 fail(s), and 0 exception(s).
[ View ]

Attached patch add https://github.com/symfony/CssSelector

Test assume only either data-id or data-class is used.

Status:Needs review» Needs work

The last submitted patch, tour-test-base-2028535-9.patch, failed testing.

clemens.tolboom’s picture

clemens.tolboom’s picture

Category:feature» bug
Status:Needs work» Needs review
StatusFileSize
new12.16 KB
new20.4 KB
PASSED: [[SimpleTest]]: [MySQL] 58,976 pass(es).
[ View ]

This is now a bug fix issue too. Patch contains #1959660: Replace xpath() with WebTestBase::cssSelect() by leveraging Symfony CssSelector and fixed some new issues.

- The test pages did not contain the all tour target (data-id) elements. Which explains the failure of #9
- The id versus data-id is unclear. These should not be the same? It is at least confusing as we have tour/id, tip/id and data-id. That's a documentation issue and for Tour UI.

https://drupal.org/files/tour-test-base-2028535-12-do-not-test.patch is without #1959660: Replace xpath() with WebTestBase::cssSelect() by leveraging Symfony CssSelector

clemens.tolboom’s picture

StatusFileSize
new20.34 KB
PASSED: [[SimpleTest]]: [MySQL] 56,362 pass(es).
[ View ]

Patch from #12 needed reroll due to #1754246: Languages should be configuration entities

The idea is test whether the target for the tips are on the page. Should we still test for the existence of the tip?

<?php
 $this
->tipOnPage($tip['id'], $selector);
?>

I added skip_ids when testing a tour.

One issue spring to mind: hook_tip_alter could add more tips on the page then function loadTours() fetches? That is hook_tour_tips_alter is not precessed by our

<?php
 
function loadTours()
?>

?

clemens.tolboom’s picture

+++ b/core/modules/tour/lib/Drupal/tour/Tests/TourTestBase.php
@@ -0,0 +1,171 @@
+  function hasTipsOnPage(array $tour, array $check_ids = array(), $skip_ids = array()) {

Needs updated documentation.

+++ b/core/modules/tour/lib/Drupal/tour/Tests/TourTestBase.php
@@ -0,0 +1,171 @@
+  function tipOnPage($tip_id, $selector) {

Needs documentation.

Next is xpath versus cssSelect:

+++ b/core/modules/tour/lib/Drupal/tour/Tests/TourTestBase.php
@@ -0,0 +1,171 @@
+    if (isset($selector[':data-id'])) {
+      $path .= '@data-id=:data-id and ';
+    }
+    if (isset($selector[':data-class'])) {
+      // No need to parse path as we select on data-class not on real class.
+      $path .= '@data-class=:data-class and ';
+    }
+    $path .= ' ./h2]';

and

+++ b/core/modules/tour/lib/Drupal/tour/Tests/TourTestBase.php
@@ -0,0 +1,171 @@
+      $path = '//*[' . '@id=:data-id' . ']';
+      $elements = $this->xpath($path, $selector);

Why not using cssSelect here? This was partly due to time to change all. see also #1959660: Replace xpath() with WebTestBase::cssSelect() by leveraging Symfony CssSelector

nick_schuch’s picture

Hi Clemens,

I have read through the patch and I think we might be able to refactor it to (instead of loading the yml file) check what tips are on the page and ensure it's elements are available.

Ill write a patch to demonstrate this more clearly but the basic idea is:

1) I go to the page.
2) I get the tips from the markup.
3) I then compare and ensure those tips have the ID on the page.

Ill continue reading through and will produce a patch.

clemens.tolboom’s picture

@nick_schuch that's a way better way.

Scanning for the available tour tips then check their targets. These are not filtered out when testing.

I'm still puzzled how to note testers on hook_tour_tips_alter as that must trigger something for the translation workflow. Should we add a failure test when running tests manually? Can we?

clemens.tolboom’s picture

Assigned:clemens.tolboom» Unassigned
xjm’s picture

Let's use a test view here instead of coupling this to the frontpage view. We can use the test views that already exist.

xjm’s picture

Title:Provide a TourBaseTest class for use by core and contrib modules» Provide a TourTestBase class for use by core and contrib modules

Also, the class name should be TourTestBase, not TourBaseTest.

nick_schuch’s picture

StatusFileSize
new5.26 KB
PASSED: [[SimpleTest]]: [MySQL] 56,754 pass(es).
[ View ]

Here is my first patch that takes the approach discussed in #15 and #16. We should also add a test for Views UI here as a proof of concept.

xjm’s picture

Issue tags:+VDC
nick_schuch’s picture

StatusFileSize
new15.76 KB
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch 2028535-22-tourtestbase-PASS.patch. Unable to apply patch. See the log in the details link for more information.
[ View ]
new15.29 KB
PASSED: [[SimpleTest]]: [MySQL] 57,030 pass(es).
[ View ]
new12.93 KB

Here is the latest patch. We NEED the cssSelect() for tip evaluation (provided in #2030369).

This patch includes a working example of the base class in action.

I have also provided a fail for the instance when no tips are found on the page in which the check is instantiated. This will help with path matching fails eg. what we have with views right now.

I have attached a FAIL and PASS patch.

Status:Needs review» Needs work

The last submitted patch, 2028535-22-tourtestbase-PASS.patch, failed testing.

larowlan’s picture

#1959660: Replace xpath() with WebTestBase::cssSelect() by leveraging Symfony CssSelector is where we're adding a cssSelect method, albeit just using the existing one provided by PHP Unit in core atm.
The Symfony one was originally proposed and rejected in that issue. #2030369: Add Symfony CssSelector to core is a duplicate in my opinion.
Looks like the tests didn't do what was expected :(

nick_schuch’s picture

Status:Needs work» Needs review
StatusFileSize
new6.69 KB
PASSED: [[SimpleTest]]: [MySQL] 57,522 pass(es).
[ View ]
new7.16 KB
FAILED: [[SimpleTest]]: [MySQL] 57,108 pass(es), 1 fail(s), and 0 exception(s).
[ View ]

Thanks larowlan!!!!

Here we go. I have also refactored the existing Views UI tour test to use our new base class.

FAIL - I am breaking the Views UI tour intentionally to show the base class failing (cannot find tips).
PASS - Checks the page for tour tips and corresponding elements.

larowlan’s picture

This looks great

+++ b/core/modules/tour/lib/Drupal/tour/Tests/TourTestBase.phpundefined
@@ -0,0 +1,63 @@
+   *   A list of tips which provide either a "data-id" or "data-class" for testing.
...
+    // Using xpath, get the rendered tips and there data-id and data-class attributes.

Needs to wrap at 80 chars

+++ b/core/modules/tour/lib/Drupal/tour/Tests/TourTestBase.phpundefined
@@ -0,0 +1,63 @@
+   *   Advanced xample:

Example

+++ b/core/modules/tour/lib/Drupal/tour/Tests/TourTestBase.phpundefined
@@ -0,0 +1,63 @@
+   *     $tips = array();
+   *     $tips[] = array('data-id' => 'foo');
+   *     $tips[] = array('data-id' => 'bar');
+   *     $tips[] = array('data-class' => 'baz');
+   *     $this->assertTourTips($tips);

Should this be wrapped in @code/@endcode?

+++ b/core/modules/tour/lib/Drupal/tour/Tests/TourTestBase.phpundefined
@@ -0,0 +1,63 @@
+      $rendered_tips = $this->xpath('//ol[@id = "tour"]//li');

Can we get a comment along the lines 'Tips are rendered as <li> elements inside <ol id="tour">' just to make it clear?

+++ b/core/modules/tour/lib/Drupal/tour/Tests/TourTestBase.phpundefined
@@ -0,0 +1,63 @@
+      $this->assertTrue(FALSE, 'Could not find tour tips on the current page.');

$this->fail() is available

+++ b/core/modules/tour/lib/Drupal/tour/Tests/TourTestBase.phpundefined
@@ -0,0 +1,63 @@
+          $elements = \PHPUnit_Util_XML::cssSelect('.' . $tip['data-class'], TRUE, $this->content, TRUE);

::cssSelect ftw

Do you think it would be possible to have another test class that extends this that looks for $this->tips and $this->paths?
Then each module could just extend that class and have the test write it self eg:

<?php
abstract class TourTestSimple extends TourTestBase {
 
  public function
testTips() {
    foreach (
$this->paths as $delta => $path) {
      if (!empty(
$this->tips[$delta])) {
       
$this->drupalGet($path);
       
$this->assertTips($this->tips['delta']);
      }
    }
  }
}

class
FooTourTest extends TourTestSimple {
    public function
getInfo(){}
    public static
modules = array('bar');
    protected
$paths = array('/foo', '/foo/bar');
    protected
$tips = array(
      array(
        array(
'data-id' => 'foo'),
        array(
'data-class' => 'bar'),
      ),
      array(
        array(
'data-id' => 'foo-bar'),
        array(
'data-class' => 'bar'),
        array(
'data-class' => 'baz'),
      ),
    );
}
?>

Thoughts?

nick_schuch’s picture

StatusFileSize
new8.42 KB
PASSED: [[SimpleTest]]: [MySQL] 57,540 pass(es).
[ View ]
new3.68 KB

That's a great idea!

Here is the patch :)

I have tested this against forum issue #1926296 with the following test class: https://gist.github.com/anonymous/a38ba00c172ac0a137d6 (it's in gist form because it's not appropriate for this issue and I will be committing it to the forum issue instead).

larowlan’s picture

Awesome, a few nitpicks

+++ b/core/modules/tour/lib/Drupal/tour/Tests/TourTestBase.phpundefined
@@ -19,21 +19,25 @@ class TourTestBase extends WebTestBase {
+   *   A list of tips which provide either a "data-id" or "data-class" ¶

Ouch whitespace

+++ b/core/modules/tour/lib/Drupal/tour/Tests/TourTestBase.phpundefined
@@ -19,21 +19,25 @@ class TourTestBase extends WebTestBase {
+    // Get the rendered tips and there data-id and data-class attributes.

their :(

+++ b/core/modules/tour/lib/Drupal/tour/Tests/TourTestBase.phpundefined
@@ -43,7 +47,7 @@ public function assertTourTips($tips = array()) {
+      $this->fail(t('Could not find tour tips on the current page.'));

Don't use t in assert messages

+++ b/core/modules/tour/lib/Drupal/tour/Tests/TourTestSimple.phpundefined
@@ -0,0 +1,68 @@
+   * An admin user with administrative permissions for views.

the tour?

+++ b/core/modules/tour/lib/Drupal/tour/Tests/TourTestSimple.phpundefined
@@ -0,0 +1,68 @@
+   * Create an admin user to view tour tips.

I think docblocks for ::setUp are frowned upon, so we should move this comment inside the function (inline comment)

+++ b/core/modules/tour/lib/Drupal/tour/Tests/TourTestSimple.phpundefined
@@ -0,0 +1,68 @@
\ No newline at end of file

Missing newline at end of file

edit: removed duplicate from dreditor

nick_schuch’s picture

StatusFileSize
new8.37 KB
PASSED: [[SimpleTest]]: [MySQL] 57,134 pass(es).
[ View ]
new2.32 KB

Here are the fixes from #29.

larowlan’s picture

More nitpicks (sorry)

+++ b/core/modules/tour/lib/Drupal/tour/Tests/TourTestBase.phpundefined
@@ -19,7 +19,7 @@ class TourTestBase extends WebTestBase {
-   *   A list of tips which provide either a "data-id" or "data-class"

Needs a .

+++ b/core/modules/tour/lib/Drupal/tour/Tests/TourTestSimple.phpundefined
@@ -65,4 +63,4 @@ public function testTips() {
\ No newline at end of file

Missing newline

nick_schuch’s picture

StatusFileSize
new8.35 KB
PASSED: [[SimpleTest]]: [MySQL] 57,409 pass(es).
[ View ]
new624 bytes

Here we go. However, I must have not included the "new line" fix in my previous patch (I have it as a commit locally). It's in the full patch.

larowlan’s picture

Status:Needs review» Reviewed & tested by the community

I think this is ready (unless bot says no)

clemens.tolboom’s picture

+++ b/core/modules/tour/lib/Drupal/tour/Tests/TourTestBase.php
@@ -0,0 +1,66 @@
+   * // Advanced example:
+   * $tips = array();

When should one use the advanced version. I'd like some more documentation here.

This relates to hook_tour_tips_alter I guess.

+++ b/core/modules/tour/lib/Drupal/tour/Tests/TourTestBase.php
@@ -0,0 +1,66 @@
+          $this->assertTrue(count($elements), format_string('Found corresponding page element for tour tip with id #%data-id', array('%data-id' => $tip['data-id'])));

iirc we tested for count($elements) === 1. Is it wrong to have more elements?

+++ b/core/modules/tour/tests/tour_test/lib/Drupal/tour_test/Controller/TourTestController.php
@@ -33,6 +33,13 @@ public function tourTest1() {
+      'tip-3' => array(
+        '#type' => 'container',

Why are these tips added? Should we document this for contrib coders?

nick_schuch’s picture

1) The advanced example is for the testing of tours that use wildcards.
2) We are testing to make sure that an element does not disappear from the page.
3) These tips are added to the "tour_test" module. Which is enabled for testing tour. We are using the "container" to provide corresponding elements on the page.

clemens.tolboom’s picture

Status:Reviewed & tested by the community» Needs work

@nick_schuch I guess I should have set this to needs works as I don't know what to do when writing a tour test. There is no documentation in the code.

Your answers from #35 should be into the patch to help 'people like me'

nick_schuch’s picture

Status:Needs work» Needs review
StatusFileSize
new8.45 KB
PASSED: [[SimpleTest]]: [MySQL] 57,472 pass(es).
[ View ]
new1.66 KB

Here is the patch as per #36.

The Tour API documentation page should be updated *after* this is committed to core. Otherwise people will start to write tests and get confused because it is not committed.

As per #28 this gist could be used for that documentation: https://gist.github.com/anonymous/a38ba00c172ac0a137d6

larowlan’s picture

Status:Needs review» Reviewed & tested by the community

unless bot disagrees, this is back to RTBC

alexpott’s picture

Status:Reviewed & tested by the community» Needs work

So we're adding an abstract class we no use case in core - this is dangerous - we have no idea if it works. There should be at least one use case - just to test it.

nick_schuch’s picture

Status:Needs work» Needs review
StatusFileSize
new6.85 KB
PASSED: [[SimpleTest]]: [MySQL] 57,302 pass(es).
[ View ]
new3.05 KB

Ok, I have removed the TourTestSimple and will implement in a different ticket which has Tour Tips to test.

I also found some additional checks for the assertTips for id's and classes (see interdiff).

larowlan’s picture

+++ b/core/modules/tour/lib/Drupal/tour/Tests/TourTestBase.phpundefined
@@ -54,11 +54,11 @@ public function assertTourTips($tips = array()) {
+          $this->assertTrue(count($elements) === 1 && $elements != FALSE, format_string('Found corresponding page element for tour tip with id #%data-id', array('%data-id' => $tip['data-id'])));

!empty($elements) && count($elements) == 1?
I had to read that a few times to make sense of it

+++ b/core/modules/tour/lib/Drupal/tour/Tests/TourTestBase.phpundefined
@@ -54,11 +54,11 @@ public function assertTourTips($tips = array()) {
+          $this->assertTrue($elements != FALSE, format_string('Found corresponding page element for tour tip with class .%data-class', array('%data-class' => $tip['data-class'])));

This would be better as assertFalse

nick_schuch’s picture

StatusFileSize
new6.85 KB
PASSED: [[SimpleTest]]: [MySQL] 57,807 pass(es).
[ View ]
new1.44 KB

Thanks larowlan!

larowlan’s picture

Status:Needs review» Reviewed & tested by the community

Unless bot disagrees

xjm’s picture

Issue tags:-VDC

#42: 2028535-42-tourtestbase.patch queued for re-testing.

Status:Reviewed & tested by the community» Needs work

The last submitted patch, 2028535-42-tourtestbase.patch, failed testing.

nick_schuch’s picture

Status:Needs work» Needs review
Issue tags:+VDC

#42: 2028535-42-tourtestbase.patch queued for re-testing.

nick_schuch’s picture

Unrelated fails.

- HTTP response expected 204, actual 400
- Value 'aQ{aB`_-' is equal to value '"9Hs>XGN

Rerunning.

Status:Needs review» Needs work

The last submitted patch, 2028535-42-tourtestbase.patch, failed testing.

larowlan’s picture

larowlan’s picture

Status:Needs work» Needs review

#42: 2028535-42-tourtestbase.patch queued for re-testing.

larowlan’s picture

Status:Needs review» Reviewed & tested by the community

Back to Rtbc

alexpott’s picture

Status:Reviewed & tested by the community» Needs work
Issue tags:+Needs tests, +Needs issue summary update

The issue summary seems to be out of date as the code example uses methods which do not exist and are not added by this patch.

+++ b/core/modules/tour/lib/Drupal/tour/Tests/TourTestBase.phpundefined
@@ -0,0 +1,67 @@
+   * // Advanced example. The following would be used for multipage or
+   * // targetting a specific subset of tips.
+   * $tips = array();
+   * $tips[] = array('data-id' => 'foo');
+   * $tips[] = array('data-id' => 'bar');
+   * $tips[] = array('data-class' => 'baz');
+   * $this->assertTourTips($tips);
+   * @endcode
+   */
+  public function assertTourTips($tips = array()) {

What's weird is that we've got usage of the advanced usage of this function. Therefore we have no idea if it works :) We need some usage in a test to confirm this works.

alexpott’s picture

Issue summary:View changes

Updated the example to patch #3

nick_schuch’s picture

Issue summary:View changes

Update summary to resemble new TourTestBase

nick_schuch’s picture

I have updated the issue summary. Will update the tests first thing tomorrow.

nick_schuch’s picture

Issue tags:-Needs tests
StatusFileSize
new7.26 KB
PASSED: [[SimpleTest]]: [MySQL] 57,587 pass(es).
[ View ]
new1.63 KB

Thanks alexpott! Here is the addition to test the advanced implementation.

nick_schuch’s picture

Status:Needs work» Needs review
nick_schuch’s picture

Issue summary:View changes

Fixes missing ?>

nick_schuch’s picture

StatusFileSize
new9.79 KB
PASSED: [[SimpleTest]]: [MySQL] 57,931 pass(es).
[ View ]
new2.96 KB

The following adds back in a class that will aid tour creators in writing tests for tip content.

I have also included an example in the description.

larowlan’s picture

Status:Needs review» Reviewed & tested by the community

I've reviewed this in length several times and I think we have the best of both worlds here.
We have a new assert helper for use in complex and simple cases.
We have a TourTestBasic class that can be easily extended where all we care about is making sure the elements we're attaching tour tips to still exist on the page, ie to protect against markup/path changes breaking existing tours.
The class name was discussed in irc and I think TourTestBasic fits far better than TourTestSimple.
There's a tour sprint at DrupalGov in around 10 days or so, getting this in before then will be a huge win.

larowlan’s picture

Issue summary:View changes

Update summary to include basic tour class example

Dries’s picture

Status:Reviewed & tested by the community» Fixed

Committed to 8.x. Thanks!

tstoeckler’s picture

Status:Fixed» Needs review
StatusFileSize
new847 bytes
PASSED: [[SimpleTest]]: [MySQL] 60,272 pass(es).
[ View ]
  1. +++ b/core/modules/tour/lib/Drupal/tour/Tests/TourTest.php
    @@ -8,12 +8,12 @@
    +use Drupal\tour\Tests\TourTestBase;

    This is actually unused.

  2. +++ b/core/modules/tour/lib/Drupal/tour/Tests/TourTestBasic.php
    @@ -0,0 +1,66 @@
    +namespace Drupal\tour\Tests;
    ...
    +use Drupal\tour\Tests\TourTestBase;

    We don't need to use something when inside the same namespace.

tstoeckler’s picture

Issue summary:View changes

Update from TourTestSimple to TourTestBasic

dawehner’s picture

Cottser’s picture

Issue summary:View changes
Status:Needs review» Reviewed & tested by the community

Follow-up looks fine!

alexpott’s picture

Status:Reviewed & tested by the community» Fixed

Committed 9279766 and pushed to 8.x. Thanks!

Status:Fixed» Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.