API page: http://api.drupal.org/api/drupal/core%21modules%21simpletest%21lib%21Dru...

Enter a descriptive title (above) relating to TestBase::generatePermutations, then describe the problem you have found:

In the example code TestBase::permute() is called, when what is meant is the function itself, i.e. TestBase::generatePermutations. This is minor, but still a bit weird.

CommentFileSizeAuthor
#16 permutations-documentation-1670312-16.patch563 bytesoriol_e9g
#6 1670312-6.patch612 bytesdsdeiz
#6 7.x-1670312-6-do-not-test.patch564 bytesdsdeiz
#2 1670312-2.patch609 bytesdsdeiz
#2 7.x-1670312-2.patch555 bytesdsdeiz
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

jhodgdon’s picture

jhodgdon
she/her
Primary language English
CreditAttribution: jhodgdon commented
Issue tags: +Novice, +Needs backport to D7

Good catch! It looks like the method name was probably changed (there is no permute() method now), and the documentation wasn't updated.

This also applies to the D7 method:
http://api.drupal.org/api/drupal/modules!simpletest!drupal_web_test_case...

dsdeiz’s picture

dsdeiz CreditAttribution: dsdeiz commented
Status: Active » Needs review
FileSize
7.x-1670312-2.patch555 bytes
1670312-2.patch609 bytes

Patch attached.

Status: Needs review » Needs work

The last submitted patch, 7.x-1670312-2.patch, failed testing.

tstoeckler’s picture

tstoeckler
he/him
Primary language German
Location Essen, Germany
CreditAttribution: tstoeckler commented
Status: Needs work » Reviewed & tested by the community

Looks good. Cool :-)

tstoeckler’s picture

tstoeckler
he/him
Primary language German
Location Essen, Germany
CreditAttribution: tstoeckler commented
Status: Reviewed & tested by the community » Needs work

Oh no, wait, sorry. I just checked, and generatePermutations() is actually a static method. So instead of
$this->generatePermutations($parameters)
we should do
TestBase::generatePermutations($parameters)

dsdeiz’s picture

dsdeiz CreditAttribution: dsdeiz commented
Status: Needs work » Needs review
FileSize
7.x-1670312-6-do-not-test.patch564 bytes
1670312-6.patch612 bytes

Patches attached.

technicalknockout’s picture

technicalknockout CreditAttribution: technicalknockout commented
Project: Drupal core » Documentation
Version: 8.x-dev »
Component: documentation » API documentation files
Status: Needs review » Needs work

Updating project field to Documentation - where the documentation code resides that needs to be patched.

technicalknockout’s picture

technicalknockout CreditAttribution: technicalknockout commented
Project: Documentation » Drupal core
Version: » 8.x-dev
Component: API documentation files » documentation

Oops, nevermind this is in the Drupal core code itself. apologies

jhodgdon’s picture

jhodgdon
she/her
Primary language English
CreditAttribution: jhodgdon commented
Status: Needs work » Reviewed & tested by the community

Very odd... Every place that generatePermutations is actually called in tests, it is called via $this->generatePermutations(), not TestBase::generatePermutations(). But it is a static method... I guess you can call it either way? Anyway, I'll get this committed to 7.x and 8.x.

jhodgdon’s picture

jhodgdon
she/her
Primary language English
CreditAttribution: jhodgdon commented

The commit will probably need to wait until this issue is fixed:
#1541958: Split setUp() into specific sub-methods

tstoeckler’s picture

tstoeckler
he/him
Primary language German
Location Essen, Germany
CreditAttribution: tstoeckler commented

Re #9: Yeah, for some reason PHP doesn't throw a fatal on that. Although IIRC it is an E_STRICT on PHP 5.3, but I guess noone runs tests with E_STRICT on 5.3.

jhodgdon’s picture

jhodgdon
she/her
Primary language English
CreditAttribution: jhodgdon commented

I wouldn't say that. When I am developing, I *always* run all my code and tests with E_STRICT. :) But I guess the test bot doesn't. :(

tstoeckler’s picture

tstoeckler
he/him
Primary language German
Location Essen, Germany
CreditAttribution: tstoeckler commented

Yes, I do too. But who has the time to run the full suite these days. :-)

jhodgdon’s picture

jhodgdon
she/her
Primary language English
CreditAttribution: jhodgdon commented

Still RTBC, but commit needs to wait on #1541958: Split setUp() into specific sub-methods

jhodgdon’s picture

jhodgdon
she/her
Primary language English
CreditAttribution: jhodgdon commented
Version: 8.x-dev » 7.x-dev
Status: Reviewed & tested by the community » Patch (to be ported)

That issue is back to 7.x, so I committed this patch to 8.x.

This patch now needs a re-roll for 7.x, but the commit to 7.x will need to wait for that issue to be fixed.
#1541958: Split setUp() into specific sub-methods

oriol_e9g’s picture

oriol_e9g
Primary language Catalan
Location Barcelona
CreditAttribution: oriol_e9g commented
Status: Patch (to be ported) » Needs review
FileSize
permutations-documentation-1670312-16.patch563 bytes
tstoeckler’s picture

tstoeckler
he/him
Primary language German
Location Essen, Germany
CreditAttribution: tstoeckler commented
Status: Needs review » Reviewed & tested by the community

Looks good.

jhodgdon’s picture

jhodgdon
she/her
Primary language English
CreditAttribution: jhodgdon commented

Thanks! I'm afraid to commit this until #1563620: All unit tests blow up with a fatal error lands, in case of a conflict... but we'll get it in sometime soon.

webchick’s picture

webchick
she/they
Primary language English
Location Vancouver 🇨🇦
CreditAttribution: webchick commented
Status: Reviewed & tested by the community » Fixed

I just checked and these two patches do not conflict.

Therefore, committed and pushed to 7.x. Thanks!

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