Split off from #2747167: Convert first part of web tests of views_ui.

The test has problems with updating radio buttons, the changes don't seem to have effect.

CommentFileSizeAuthor
#13 2864610-13.patch4.56 KBManuel Garcia
#13 interdiff.txt2.82 KBManuel Garcia
#9 2864610-4_1381140-129_combined.patch11.04 KBManuel Garcia
#7 Screenshot from 2017-08-04 10-54-40.png4.78 KBManuel Garcia
#3 2864610-3.patch982 bytesandypost
#4 interdiff.txt3.91 KBandypost
#4 2864610-4.patch4.55 KBandypost
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

michielnugter created an issue. See original summary.

michielnugter’s picture

michielnugter CreditAttribution: michielnugter as a volunteer and at Synetic commented
Component: phpunit » views_ui.module
Issue tags: +phpunit initiative
andypost’s picture

andypost
he/him
Primary language Russian
CreditAttribution: andypost as a volunteer and at Skilld commented
Status: Active » Needs review
FileSize
2864610-3.patch982 bytes

initial patch, does not pass locally

andypost’s picture

andypost
he/him
Primary language Russian
CreditAttribution: andypost as a volunteer and at Skilld commented
FileSize
2864610-4.patch4.55 KB
interdiff.txt3.91 KB

fixed few assertions, somehow result value is different

Status: Needs review » Needs work

The last submitted patch, 3: 2864610-3.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.

Manuel Garcia’s picture

Manuel Garcia CreditAttribution: Manuel Garcia as a volunteer and at Appnovation commented
FileSize
Screenshot from 2017-08-04 10-54-40.png4.78 KB

I've had a look at the failing test, but could not figure out why it fails. Here some of my findings / thoughts:

\Drupal\FunctionalTests\AssertLegacyTrait::assertFieldChecked() works differently than \Drupal\simpletest\AssertContentTrait::assertFieldChecked() so that might be why these fail, or it might be a legitimate failure?

I had a look at the html produced by the test, and checked="checked" is pressent on the right elements, although when its rendered it looks like this:

Lendude’s picture

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

@Manuel Garcia Ha, you ran into my favourite Drupal bug! #1381140: A #default_value = 0 for #type radios checks all radios

Manuel Garcia’s picture

Manuel Garcia CreditAttribution: Manuel Garcia as a volunteer and at Appnovation commented
Status: Needs work » Needs review
FileSize
2864610-4_1381140-129_combined.patch11.04 KB

Ow wow, so it is a legitimate bug - thanks @Lendude for the info!

I went ahead and tested #4 patch with #1381140: A #default_value = 0 for #type radios checks all radios (#129) applied, and it passes, so that's great news. Uploading the combined patch here just to make sure.

Lendude’s picture

Lendude
Primary language Dutch
Location Amsterdam
CreditAttribution: Lendude as a volunteer and at Dx Experts commented
Status: Needs review » Postponed
Related issues: +#1381140: A #default_value = 0 for #type radios checks all radios

#9 is pretty clear, lets postpone on #1381140: A #default_value = 0 for #type radios checks all radios

Manuel Garcia’s picture

Manuel Garcia CreditAttribution: Manuel Garcia as a volunteer and at Appnovation commented
Status: Postponed » Needs review

#1381140: A #default_value = 0 for #type radios checks all radios landed yay!

Manuel Garcia’s picture

Manuel Garcia CreditAttribution: Manuel Garcia as a volunteer and at Appnovation commented

Re-testing #4...

Manuel Garcia’s picture

Manuel Garcia CreditAttribution: Manuel Garcia as a volunteer and at Appnovation commented
FileSize
interdiff.txt2.82 KB
2864610-13.patch4.56 KB

This should hopefully come back green =)

Lendude’s picture

Lendude
Primary language Dutch
Location Amsterdam
CreditAttribution: Lendude as a volunteer and at Dx Experts commented
Status: Needs review » Reviewed & tested by the community

Nice.

The remaining tests in Views UI have separate issues, so the scope here is correct.

  • catch committed 7bf7ceb on 8.5.x 
    Issue #2864610 by Manuel Garcia, andypost, Lendude: Convert...

  • catch committed 5d9f959 on 8.4.x 
    Issue #2864610 by Manuel Garcia, andypost, Lendude: Convert...
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: Reviewed & tested by the community » Fixed

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

Status: Fixed » Closed (fixed)

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