Once we finalize the API changes, what I'd love to see is a backwards compatibility layer around the 5.x and 6.x versions of SimpleTest, so that for example in a 5.x module's .test file I could write either:

function get_info() { // Deprecated.
    return array(
      'name' => t('Privatemsg message send'),
      'desc' => t('Send a message and check that it landed.'),
      'group' => 'Privatemsg',
    );
}

or:

function getInfo() {
    return array(
      'name' => t('Privatemsg message send'),
      'description' => t('Send a message and check that it landed.'),
      'group' => 'Privatemsg',
    );
}

... and it would automatically call the right function.

This would make it much easier for the vast majority of developers who are /not/ working on core, and are instead working on their own contrib modules to use the same syntax as the people working on 7.x can use, while not breaking any existing tests.

Without this, developers have to really learn two versions of the API, and that's going to slow down test adoption. If we can get people to write tests for their own modules, we should also make the shift to writing tests for core as easy as possible.

CommentFileSizeAuthor
#24 simpletest-backwards-compatibility.patch44.31 KBboombatower
#10 simpletest-backwards-compatibility-241156-10.patch40.87 KBwebchick
#8 simpletest-backwards-compatibility-241156-8.patch40.89 KBwebchick

Comments

webchick’s picture

webchick
she/they
Primary language English
Location Vancouver 🇨🇦
commented
Status: Postponed » Active

Now that SimpleTest module is in core, let's solve this nightmare; atm, people need to learn two different syntaxes for 6.x tests vs. 7.x tests. This is a huge barrier to getting more people involved in testing.

Changes I've found so far:
- class DrupalTestCase is now DrupalWebTestCase
- get_info() is now getInfo()
- 'desc' in getInfo() is now 'description'

boombatower’s picture

boombatower commented

I plan to work on this soon since I am aware of the majority of the changes.

boombatower’s picture

boombatower commented
Status: Active » Postponed (maintainer needs more info)

To make sure we are on same page.

I think what you would like is to allow developers to write 7.x syntax tests in 5.x and 6.x version of module. Not allowing 5.x and 6.x syntax in 7.x.

Is that correct?

webchick’s picture

webchick
she/they
Primary language English
Location Vancouver 🇨🇦
commented
Status: Postponed (maintainer needs more info) » Active

Yep, absolutely correct. 7.x's syntax should be the "one true" syntax, but at the same time we don't want to break existing tests in 5.x/6.x. So 5.x/6.x version of SimpleTest should allow me to write my tests the "new" way without screwing up any tests already written.

webchick’s picture

webchick
she/they
Primary language English
Location Vancouver 🇨🇦
commented
Assigned: Unassigned » webchick

Ok, this is my itch of the day.

webchick’s picture

webchick
she/they
Primary language English
Location Vancouver 🇨🇦
commented

I was thinking about this out loud in #drupal and it kind of hit me... solving this could almost be as simple as putting drupal_web_test_case.php in 6.x and 5.x simpletest module. Old tests would inherit from DrupalTestCase which uses the old syntax, new tests would inherit from DrupalWebTestCase which would have the new stuff.

Probably still need to futz some with get_info vs. getInfo() but overall, hopefully this'll be pretty easy to pull off.

boombatower’s picture

boombatower commented

Only three references to get_info.

  1. _addClassToGroups in drupal_unit_tests.php
  2. drupal_test_case.php line 23-24, 513-514
  3. simpletest.module line 146

As for simply adding the drupal_web_test_case.php, that would work, but you would probably want to remove a few things and discuss a few others.

  • Do we want the whole database prefixing stuff and is it possible using the install systems in 5.x and 6.x?
  • Test files stuff - if so we need to add .install code for them.

I'm sure a few other things may arise, but those are the ones that come to mind of the top of my head.

webchick’s picture

webchick
she/they
Primary language English
Location Vancouver 🇨🇦
commented
Status: Active » Needs review
StatusFileSize
new simpletest-backwards-compatibility-241156-8.patch40.89 KB

Holy crap, I think it's working. Well, at least enough so there are no fatal errors. :P Any idea if there are tests out there that pass in both 6.x and 7.x? So far I haven't found any that pass in 6.x alone. :(

webchick’s picture

webchick
she/they
Primary language English
Location Vancouver 🇨🇦
commented
Status: Needs review » Needs work

Oops. The requirements patch was just committed, so this won't work anymore. ;) Re-roll coming up.

webchick’s picture

webchick
she/they
Primary language English
Location Vancouver 🇨🇦
commented
Status: Needs work » Needs review
StatusFileSize
new simpletest-backwards-compatibility-241156-10.patch40.87 KB

Here we go.

webchick’s picture

webchick
she/they
Primary language English
Location Vancouver 🇨🇦
commented

@ * Do we want the whole database prefixing stuff and is it possible using the install systems in 5.x and 6.x?
* Test files stuff - if so we need to add .install code for them.

Eh. I'm ambivalent on this. There's a lot of nice stuff in 7.x (the UI tweaks for one thing) but at the moment the only thing I'm worried about is the syntax matching so that we don't need to write two sets of docs.

I'd support this stuff getting included, but separate from this patch. This patch should deal only with the .test syntax.

webchick’s picture

webchick
she/they
Primary language English
Location Vancouver 🇨🇦
commented
Status: Needs review » Needs work

Ok, I finally found a test that passes in 7.x: block module: 58 passes, 0 fails and 0 exceptions.

When I copy block.test into my 6.x install though, I get 44 passes, 35 fails and 0 exceptions.

So something's still messed up.

webchick’s picture

webchick
she/they
Primary language English
Location Vancouver 🇨🇦
commented

So what's going on is when it attempts to fill in the add block form, instead of the form, it's getting back an access denied error.

drupalLogin() appears to be working correctly. Trying to dig in further and see if I can figure out where it's coming from.

webchick’s picture

webchick
she/they
Primary language English
Location Vancouver 🇨🇦
commented
Status: Needs work » Needs review

Oooo! Actually, after doing a clean install (my 6.x testbed is a bunch of a mess atm), that brought block.test down to 58 passes, 10 fails and 0 exceptions. The first failure is:

Found the requested form at [/Applications/MAMP/htdocs/6x2/sites/all/modules/simpletest/drupal_web_test_case.php line 566] FAIL

However, curiously:

Found the Save blocks button at [/Applications/MAMP/htdocs/6x2/sites/all/modules/simpletest/drupal_web_test_case.php line 567] OK

So it sees the form, just not the form element it's looking for...

Upon further inspection, testBlock has the following:

    // Select the Navigation block to be configured and moved.
...
    $block['delta'] = 'navigation';

In HEAD, a patch recently went in: #216072: DROP Task: Remove hardcoded numeric deltas from all hook_block() implementations in core

By changing that line to:

$block['delta'] = 1;

Suddenly we're back at:

62 passes, 0 fails and 0 exceptions.

WOOHOO! :)

Marking back for review.

webchick’s picture

webchick
she/they
Primary language English
Location Vancouver 🇨🇦
commented

I should point out too that block.test is a fairly good one for exercising that this back-port was in fact successful. It contains:

- New getInfo() function (used to be get_info())
- New 'description' parameter of getInfo() (used to be 'desc')
- A call to $this->drupalCreateUser() (used to be $this->drupalCreateRolePerm())
- A call to $this->drupalLogin() (used to be $this->drupalLoginUser())

However, I'd still feel better about getting a couple extra ones to confirm. The challenge is finding 7.x tests that pass where the underlying system hasn't changed much between 6.x and 7.x. :(

webchick’s picture

webchick
she/they
Primary language English
Location Vancouver 🇨🇦
commented

Hm. book.test passes in 7.x, gives me 52 passes, 50 fails and 55 exceptions in 6.x. Same deal where the permissions don't seem to 'take', but this time it wasn't cleared up with a fresh install.

I'm still leaving for review though in case someone has any bright ideas as to why that isn't passing.

pwolanin’s picture

pwolanin commented

subscribing

boombatower’s picture

boombatower commented

I will take a look at this in more detail soon, but as a comment off the top of my head.

Does this patch include http://drupal.org/node/205892? If not I think it is somewhat related and could be rolled into this patch.

soxofaan’s picture

soxofaan commented

subscribing

boombatower’s picture

boombatower commented

The 7.x tests don't clean up after themselves since they can count of the database being flushed.

That may cause some of the issues you are seeing.

boombatower’s picture

boombatower commented
Status: Needs work » Needs review

Looking at the code is seems fine. I ran the block test and got all passes.

I would attempt to getting the database stuff to work if that isn't to hard in 6.x system.

Since this patch changes the get_info stuff anyway it would be nice if it included http://drupal.org/node/205892.

I'm thinking this should included the requirements hook for CuRL and stuff like previous issue?

boombatower’s picture

boombatower commented
Status: Needs review » Needs work
boombatower’s picture

boombatower commented
Status: Needs review » Needs work

Currently fixing this patch up, to avoid conflicts.

boombatower’s picture

boombatower commented
Status: Needs work » Needs review
StatusFileSize
new simpletest-backwards-compatibility.patch44.31 KB

This should provide database prefixing and temporary directories for each test method.

Once this is committed I will compare with 7.x HEAD and back-port a few other changes.

As a note there are several changes made to Drupal 7.x core to make SimpleTest more seamless...those changes won't be in 6.x and will cause tests like BlogAPI not to work.

We can provide patches to the core a notes in the test files that say apply these patches if you want to run these, or just not include those tests.

webchick’s picture

webchick
she/they
Primary language English
Location Vancouver 🇨🇦
commented

Simpletest
Please add the following code to the bottom of settings.php

$GLOBALS["simpletest_ua_key"] = 255162;
if (preg_match("/^(simpletest\d+),(\d+)/", $_SERVER["HTTP_USER_AGENT"], $matches) && $GLOBALS["simpletest_ua_key"] == $matches[2]) {
$db_prefix = $matches[1];
}

Check. :)

Re-ran block.test from #259985: block.test for 6.x. Still passes! Check.

Ran the 7.x version of book.test. 10 failures. :( But still, that's better than 50. ;) They all deal with previous/next links, so I'm looking to see what might've changed with that in 7.x.

webchick’s picture

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

OMG! Book.test passes! I had just saved it stupidly so the little arrows got messed up.

This is EXCITING!

I'll try a couple more, but assuming I don't find any other issues, RTBC. :)

webchick’s picture

webchick
she/they
Primary language English
Location Vancouver 🇨🇦
commented

contact.test passes also, with no modifications. dblog.test has some failures (10 of which relate to 'article' vs. 'story'), but that test has always been a little janky, so I'm not too worried. forum.test also has failures, I didn't have time to look into them.

I still think we should commit this, even as an interim step, since there are definitely tests there that *do* pass, we must be hitting at least 60-70% of the use cases. And for the weird ones, better to have 50 people writing tests and debugging this than 2 people. ;)

boombatower’s picture

boombatower commented
Status: Reviewed & tested by the community » Fixed

Committed.

I will compare with 7.x branch a bit more and probably port back a few other changes.

I will marked this fixed for now (unless that doesn't seem fitting). Once I port it back I will mark this as too be ported.

So post all other patches to this issue.

webchick’s picture

webchick
she/they
Primary language English
Location Vancouver 🇨🇦
commented
Title: Provide backwards-compatibility layer for 5.x/6.x simpletest » Provide backwards-compatibility layer for 5.x simpletest
Version: 6.x-1.x-dev » 5.x-1.x-dev
Assigned: webchick » Unassigned
Priority: Critical » Normal
Status: Fixed » Patch (to be ported)

Actually that issue title is misleading, since this didn't get ported to 5.x.

This isn't on my priority list, as I'm doing development in 6, but would be a great thing for someone to do if they had time.

jadelrab’s picture

jadelrab commented

subscribing

boombatower’s picture

boombatower commented
Status: Patch (to be ported) » Closed (won't fix)

No maintained.