Support from Acquia helps fund testing for Drupal Acquia logo

Comments

tim.plunkett’s picture

Status: Active » Needs review
FileSize
2.42 KB

See attached

boombatower’s picture

Yea, as I suggested committing this first will make it easier to be sure the pifr change works before we commit the actual core change so we don't end up with the disruption we had yesterday with the commit.

sun’s picture

I can't RTBC my own patch.

tim.plunkett’s picture

Status: Needs review » Reviewed & tested by the community

I can do that. (I should have mentioned above, I didn't write a single line of this, sun did).

Dries’s picture

It would be nicer if the command line settings where consistent with the settings in the Simpletest configuration UI.

I prefer --keep-results over --clear-results so maybe we should consider renaming the internal variable to simpletest_clear_results to simpletest_keep_results and updating the configuration UI accordingly? It would also make the code a tiny bit cleaner.

Dries’s picture

Status: Reviewed & tested by the community » Fixed

I don't want to hold up this issue on #5 so I decided to go ahead and commit it.

If people feel #5 is valuable, maybe we can create a follow-up patch or a new issue? Marking this issue 'fixed' for the time being.

sun’s picture

The default, in both cases, should actually be to delete the results. Keeping the assertion results should be the opt-in, not the other way around.

So I'd be fine with patching the Simpletest module to rename its variable/config-key from clear_results to keep_results (as well as adjusting the UI and negating the default value).

sun’s picture

Version: 8.x-dev » 7.x-dev
Status: Fixed » Patch (to be ported)
Issue tags: +Needs backport to D7

We actually need to backport this to D7...

sun’s picture

Version: 7.x-dev » 8.x-dev
Status: Patch (to be ported) » Needs review
FileSize
952 bytes

Oh dear. I'm sorry.

boombatower’s picture

was just going to post that I confirmed it was this from the other issue. testing.

boombatower’s picture

Status: Needs review » Reviewed & tested by the community

Confirmed locally.

webchick’s picture

Version: 8.x-dev » 7.x-dev
Status: Reviewed & tested by the community » Fixed

Oops. :)

Committed and pushed to 8.x. Thanks!

Moving back to 7.x.

webchick’s picture

Status: Fixed » Patch (to be ported)

Ahem.

sun’s picture

Issue tags: +Testing system
sun’s picture

Version: 7.x-dev » 8.x-dev
Status: Patch (to be ported) » Reviewed & tested by the community
FileSize
1.02 KB

Finally discovered why we're getting lots of verbose output in test results -- which lead to many more total test result assertions, and since PIFR has a maximum limit of assertions it displays, this means that actual errors and exceptions of failing tests are not displayed. (example)

This is a stop-gap fix for D8, which can be removed with #1705748: Convert simpletest settings to configuration system

webchick’s picture

Status: Reviewed & tested by the community » Fixed

Heh, oops. Since SimpleTest hasn't been converted to CMI yet, these settings currently have no effect. The temporary patch adds them back in under their old D7 names as a stop-gap.

Committed and pushed to 8.x.

sun’s picture

Version: 8.x-dev » 7.x-dev
Status: Fixed » Patch (to be ported)

Thanks! :)

boombatower’s picture

boombatower’s picture

For future reference note that this issue also change the definition of --verbose to include controlling the simpletest_verbose variable which controls the capturing the browser requests in DWTC.

To be fixed in one way or another in
#1774002: Correct mistake introduced in simpletest cmi conversion by introducing --verbose-browser flag

webchick’s picture

Priority: Major » Normal

Correct me if I'm wrong, but I don't believe this backport is actually major for 7.x; it merely keeps the code bases closer together. Tentatively de-prioritizing so that I can commit me some features. :)

dcam’s picture

Which of these patches should be backported? I ask because #15 is a "stop-gap fix for D8."

webchick’s picture

Version: 7.x-dev » 8.x-dev
Status: Patch (to be ported) » Postponed (maintainer needs more info)

Actually, that is a fine question.

sun’s picture

Version: 8.x-dev » 7.x-dev
Status: Postponed (maintainer needs more info) » Patch (to be ported)

There are three patches in this issue that have been committed, and the cumulative result of all three of them should be backported.

dcam’s picture

Status: Patch (to be ported) » Needs review
FileSize
2.68 KB

Backported #1, #9, and #15 to D7.

Status: Needs review » Needs work

The last submitted patch, test-keep-results-1719530-24.patch, failed testing.

dcam’s picture

Status: Needs work » Needs review
FileSize
2.67 KB

Fixed the filepath in line 410 of run-tests.sh that was causing the error in #24.

boombatower’s picture

It would seem prudent to get #1774002: Correct mistake introduced in simpletest cmi conversion by introducing --verbose-browser flag in and add to backport since the patch as it stands introduces an "api change" by changing the meaning of --verbose instead of just two additional flags (only adds one the way it stands and alters meaning of the other).

alberto56’s picture

Here is a version of #26 which applies to 7.22.

  • Dries committed 2617260 on 8.3.x
    - Patch #1719530 by tim.plunkett: Add --keep-results flag to run-tests....
  • webchick committed 5819f5e on 8.3.x
    Issue #1719530 by tim.plunkett, sun: Add --keep-results flag to run-...
  • webchick committed aa9744b on 8.3.x
    Issue #1719530 follow-up by sun: SimpleTest hasn't been converted to CMI...

  • Dries committed 2617260 on 8.3.x
    - Patch #1719530 by tim.plunkett: Add --keep-results flag to run-tests....
  • webchick committed 5819f5e on 8.3.x
    Issue #1719530 by tim.plunkett, sun: Add --keep-results flag to run-...
  • webchick committed aa9744b on 8.3.x
    Issue #1719530 follow-up by sun: SimpleTest hasn't been converted to CMI...

  • Dries committed 2617260 on 8.4.x
    - Patch #1719530 by tim.plunkett: Add --keep-results flag to run-tests....
  • webchick committed 5819f5e on 8.4.x
    Issue #1719530 by tim.plunkett, sun: Add --keep-results flag to run-...
  • webchick committed aa9744b on 8.4.x
    Issue #1719530 follow-up by sun: SimpleTest hasn't been converted to CMI...

  • Dries committed 2617260 on 8.4.x
    - Patch #1719530 by tim.plunkett: Add --keep-results flag to run-tests....
  • webchick committed 5819f5e on 8.4.x
    Issue #1719530 by tim.plunkett, sun: Add --keep-results flag to run-...
  • webchick committed aa9744b on 8.4.x
    Issue #1719530 follow-up by sun: SimpleTest hasn't been converted to CMI...

  • Dries committed 2617260 on 9.1.x
    - Patch #1719530 by tim.plunkett: Add --keep-results flag to run-tests....
  • webchick committed 5819f5e on 9.1.x
    Issue #1719530 by tim.plunkett, sun: Add --keep-results flag to run-...
  • webchick committed aa9744b on 9.1.x
    Issue #1719530 follow-up by sun: SimpleTest hasn't been converted to CMI...