Problem/Motivation

See #2735005: Convert all Simpletest web tests to BrowserTestBase (or UnitTestBase/KernelTestBase).

Test Drupal\history\Tests\Views\HistoryTimestampTest was not converted in #2795049: Convert web tests to browser tests for history module

Proposed resolution

Convert it

CommentFileSizeAuthor
#15 2898437-history-ktb-15.patch2.72 KBandypost
#15 interdiff-2898437-13.patch929 bytesandypost
#13 2898437-history-ktb-12.patch2.71 KBandypost
#13 interdiff-2898437-8.txt1.02 KBandypost
#8 2898437-history-ktb-8.patch2.93 KBandypost
#8 interdiff-2898437-6.txt889 bytesandypost
#6 2898437-history-ktb-6.patch2.9 KBandypost
#2 2898437-history-bt-2.patch1.5 KBandypost
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

andypost created an issue. See original summary.

andypost’s picture

andypost
he/him
Primary language Russian
CreditAttribution: andypost as a volunteer and at Skilld commented
Assigned: andypost » Unassigned
Status: Active » Needs review
FileSize
2898437-history-bt-2.patch1.5 KB

Here it is

dawehner’s picture

dawehner
Primary language German
CreditAttribution: dawehner at Chapter Three commented

@andypost
If you look at the patch we could also convert it to a kernel test, in which case, we maybe even have to change less. Do you think this would be a good change?

andypost’s picture

andypost
he/him
Primary language Russian
CreditAttribution: andypost as a volunteer and at Skilld commented
Status: Needs review » Needs work

@dawehner thanx! good idea & test could be faster

andypost’s picture

andypost
he/him
Primary language Russian
CreditAttribution: andypost as a volunteer and at Skilld commented
Status: Needs work » Needs review

No, this require to add setup schemas & create entities manually

So this is a minimal changes(

But if it makes sense I could convert

andypost’s picture

andypost
he/him
Primary language Russian
CreditAttribution: andypost as a volunteer and at Skilld commented
FileSize
2898437-history-ktb-6.patch2.9 KB

Here it is, also fixed "@see" to point actual view plugins

Lendude’s picture

Lendude
Primary language Dutch
Location Amsterdam
CreditAttribution: Lendude as a volunteer and at Dx Experts commented

kerneltest++

+++ b/core/modules/history/tests/src/Kernel/Views/HistoryTimestampTest.php
@@ -29,15 +31,40 @@ class HistoryTimestampTest extends ViewTestBase {
+    // Use classy theme because only its marker cold be found with xpath.

cold => could. And any idea why that is?

The rest looks great.

andypost’s picture

andypost
he/him
Primary language Russian
CreditAttribution: andypost as a volunteer and at Skilld commented
Title: Convert HistoryTimestampTest to browser test » Convert HistoryTimestampTest to kernel test
FileSize
interdiff-2898437-6.txt889 bytes
2898437-history-ktb-8.patch2.93 KB

Elaborated comment, all other templates has no span wrapper for marker but the test using following xpath

    $result = $this->xpath('//span[@class=:class]', [':class' => 'marker']);
    $this->assertEqual(count($result), 1, 'Just one node is marked as new');
dawehner’s picture

dawehner
Primary language German
CreditAttribution: dawehner at Chapter Three commented 
+++ b/core/modules/history/tests/src/Kernel/Views/HistoryTimestampTest.php
@@ -1,25 +1,27 @@
+  public static $modules = ['history', 'node', 'filter', 'user'];

To be honest I don't understand why we need the filter module. What is the reason for that, I'm just curious?

Status: Needs review » Needs work

The last submitted patch, 8: 2898437-history-ktb-8.patch, failed testing. View results

tacituseu’s picture

tacituseu CreditAttribution: tacituseu commented

Segmentation fault (core dumped)

Lendude’s picture

Lendude
Primary language Dutch
Location Amsterdam
CreditAttribution: Lendude as a volunteer and at Dx Experts commented

#9 we don't, just ran it without and it passes fine:

Time: 17.18 seconds, Memory: 6.00MB

OK (1 test, 8 assertions)

Process finished with exit code 0


so lets take that out.

Comment is much clearer but needs a little different syntax I think

+++ b/core/modules/history/tests/src/Kernel/Views/HistoryTimestampTest.php
@@ -29,15 +31,41 @@ class HistoryTimestampTest extends ViewTestBase {
+    // Use classy theme because only its marker wrappend in span so could be

I would go for something like: Use classy theme because its marker is wrapped in a span so it can be easily targeted with xpath.

andypost’s picture

andypost
he/him
Primary language Russian
CreditAttribution: andypost as a volunteer and at Skilld commented
Status: Needs work » Needs review
FileSize
interdiff-2898437-8.txt1.02 KB
2898437-history-ktb-12.patch2.71 KB

I called retest, fail unrelated

@dawehner cleaned-up that, looks that no longer needed for node module (node -> text -> filter)

The last submitted patch, 8: 2898437-history-ktb-8.patch, failed testing. View results

andypost’s picture

andypost
he/him
Primary language Russian
CreditAttribution: andypost as a volunteer and at Skilld commented
FileSize
interdiff-2898437-13.patch929 bytes
2898437-history-ktb-15.patch2.72 KB

Fixed code comment suggestion from #12

PS: sadly interdiff with ".patch" extension

dawehner’s picture

dawehner
Primary language German
CreditAttribution: dawehner at Chapter Three commented
Status: Needs review » Reviewed & tested by the community

I think we are good now. Great work @andypost!

The last submitted patch, 15: interdiff-2898437-13.patch, failed testing. View results

Version: 8.4.x-dev » 8.5.x-dev

Drupal 8.4.0-alpha1 will be released the week of July 31, 2017, which means new developments and disruptive changes should now be targeted against the 8.5.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

  • catch committed 3e6a3f0 on 8.5.x 
    Issue #2898437 by andypost, dawehner, Lendude: Convert...

  • catch committed 4130423 on 8.4.x 
    Issue #2898437 by andypost, dawehner, Lendude: Convert...
catch’s picture

catch
he/him
Primary language English
CreditAttribution: catch at Third and Grove commented
Status: Reviewed & tested by the community » Fixed

Committed/pushed to 8.5.x and cherry-picked to 8.4.x. Thanks!

catch’s picture

catch
he/him
Primary language English
CreditAttribution: catch at Third and Grove commented
Version: 8.5.x-dev » 8.4.x-dev

Status: Fixed » Closed (fixed)

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