In CheckboxTest::testFormCheckbox() there are two loops over XPath queries. These loops never actually execute because JSON is returned instead of HTML.

CommentFileSizeAuthor
#4 2667358-4.patch3.77 KBtwistor
#2 2667358-2.patch1.91 KBtwistor
#2 2667358-2-test-only-should-fail.patch1.16 KBtwistor
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

twistor created an issue. See original summary.

twistor’s picture

twistor CreditAttribution: twistor as a volunteer commented
Status: Active » Needs review
FileSize
2667358-2-test-only-should-fail.patch1.16 KB
2667358-2.patch1.91 KB

The last submitted patch, 2: 2667358-2-test-only-should-fail.patch, failed testing.

twistor’s picture

twistor CreditAttribution: twistor as a volunteer commented
FileSize
2667358-4.patch3.77 KB

I think it would be better to be explicit with the json parameter. No need for defaults.

Version: 8.0.x-dev » 8.1.x-dev

Drupal 8.0.6 was released on April 6 and is the final bugfix release for the Drupal 8.0.x series. Drupal 8.0.x will not receive any further development aside from security fixes. Drupal 8.1.0-rc1 is now available and sites should prepare to update to 8.1.0.

Bug reports should be targeted against the 8.1.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.2.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

daffie’s picture

daffie CreditAttribution: daffie commented
Version: 8.1.x-dev » 8.2.x-dev
Status: Needs review » Reviewed & tested by the community

I have reviewed the patch and it all looks good to me.

1. The adding of the explicit json parameter is making the test more clear to me.

2. The change from has('json') to get('json') looks a bit strange to me. But with the double test patch from comment #2 proves that the change is good and necessary.

Good work twistor.

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
CreditAttribution: alexpott at Chapter Three commented
Status: Reviewed & tested by the community » Fixed

Committed f813708 and pushed to 8.1.x and 8.2.x. Thanks!

  • alexpott committed 05da917 on 8.2.x 
    Issue #2667358 by twistor: CheckboxTest doesn't test what it thinks it's...

Status: Fixed » Closed (fixed)

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