The current method for grouping tests, which currently only consist of functional tests, simple groups them per module (or similarly). Once we add 150 unit tests it will be unclear how to group them since 53 of them will be related to include files not modules. The other 97 will be related to the individual files of each core module.

Creating groups for each of the include files or related systems coupled with the complete functional test coverage will overload the administration page. I think we need to come up with a better way to group the files, maybe only for unit testing include files.

Another helpful addition might be to create tabs for "Unit" and "Functional" testing on the page with the list of tests.

CommentFileSizeAuthor
#20 group_naming_convention.patch22.86 KBboombatower
#19 split_tests_by_type.patch3.52 KBboombatower
#17 simpletest_grouping.png21.78 KBboombatower
#16 simpletest_group_convention_revised.patch8.26 KBboombatower
#15 simpletest_group_convention_revised.patch7.7 KBboombatower
#11 simpletest_group_convention_revised.patch6.68 KBboombatower
#12 simpletest_group_convention_revised_1.patch7.65 KBdlangevin
#10 simpletest_group_convention_revised.patch7.38 KBdlangevin
#8 simpletest_group_convention.patch5.94 KBdlangevin
#4 simpletest_group_convention.patch3.97 KBboombatower
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

boombatower’s picture

boombatower CreditAttribution: boombatower commented

I have noticed that the functional list, which still isn't complete, is becoming hard to scan due to its length.

I propose that tabs be created, one for Unit Testing and one for Functional Testing. That would provide a basis for the list to be shortened.

Next I think we should get rid of the fieldsets for grouping and move towards something more like a list. The menu tree is displayed in a list-like fashion that we could build on. The sub-tests could be displayed indented and the checkbox functionality would still remain.

The list would be more compact and easier to read.

This might also alleviate: http://drupal.org/node/214570.

birdmanx35’s picture

birdmanx35 CreditAttribution: birdmanx35 commented

I think tabs are a good idea, but perhaps vertical tabs, and not horizontal tabs? I feel like that's generally more usable, and more noticeable.

Fieldsets are also bad, so +1 on the list idea.

Mockups?

boombatower’s picture

boombatower CreditAttribution: boombatower commented

I don't think vertical tabs would be useful since only two tabs and list is very long so vertical space would be a waste.

As for mockups I could, but it will look like menu page. So that should give an idea.

boombatower’s picture

boombatower CreditAttribution: boombatower commented
Version: 6.x-1.x-dev »
Status: Active » Needs review
FileSize
simpletest_group_convention.patch3.97 KB

Here is an initial patch. This isn't fully functioning in the JavaScript area (ie. collapse and "check all"/"check group") and will no doubt need a bit a cleanup.

The patch doesn't add tabs yet and after a test is run the results page hasn't been overridden.

This is a preliminary patch to allow for feedback on the approach.

beeradb’s picture

beeradb CreditAttribution: beeradb commented
Status: Needs review » Needs work

We definitely need to do something about this, as the current interface is pretty blah. I think even moving to a table based layout would be a improvement, as each test takes up way more screen real estate than it should right now. That being said, this patch needs some work.

What about two different tabs for functional/unit tests.. then for each tab have a UL listing the test groups, with individual tests for each test group listed within a collapsible sub ul? In addition, we should alphabetize the current list, as it's pretty annoying trying to find what you're looking for.

Example:

  • [ ] Aggregator Tests
    • [ ] Add Feed Functionality
    • [ ] Update Feed Functionality
    • [ ] Remove Feed Functionality
    • [ ] ...

Having the internal UL collapsed by default. I think this, combined with tabs, and alphabetizing the list, will help a lot.

boombatower’s picture

boombatower CreditAttribution: boombatower commented

I had already planned on tabs as mentioned above. The current table layout was planned to collapse, but I wasn't done.

A list would work, should be someone simple looking.

I would like further feedback if possible. I think we all agree that it takes up too much space and that it should optimized and groups should remain collapsible. I guess at this point the only question is what format? List? Table? Other?

dlangevin’s picture

dlangevin CreditAttribution: dlangevin commented

When I added this patch to simpletest module running on 7.x head, the theme function wasn't being called. Anyone else with this problem?

UPDATED:

Oops... just had to delete the cache

dlangevin’s picture

dlangevin CreditAttribution: dlangevin commented
Status: Needs work » Needs review
FileSize
simpletest_group_convention.patch5.94 KB

I saw what you did an it looked good. Added in the JS to expand/contract the trs please review

I think that tabs might be a little difficult to use if you're running lots of tests at the same time

boombatower’s picture

boombatower CreditAttribution: boombatower commented
Status: Needs review » Needs work

I was hopping someone would come along and do that. :)

One problem the patch doesn't seem to be valid. Please see: http://drupal.org/patch.

dlangevin’s picture

dlangevin CreditAttribution: dlangevin commented
Status: Needs work » Needs review
FileSize
simpletest_group_convention_revised.patch7.38 KB

sorry. That was my first patch, so I think I did something wrong with the formatting. I re-rolled it following the instructions. Please let me know if it's OK now.

boombatower’s picture

boombatower CreditAttribution: boombatower commented
Status: Needs review » Needs work
FileSize
simpletest_group_convention_revised.patch6.68 KB

@dlangevin: Great work!

Only one issue, since SimpleTest is being developed so rapidly it is a good idea to update right before you create a patch. The patch reverted a few other commits, but that was very easy to fix.

Thanks for taking initiative. This is looking good.

@all: I cleaned up the code a bit more and added labels on the test names so that they will relate to the checkboxes.

I think the only things that remain are:

  • Fix column widths when tests are not shown. The description and run columns look kinda wacky when no groups are expanded.
  • Remove the automatic expansion of a group when the "check all" checkbox is checked.
  • Possibly fix the coloring of the group rows. The even-odd coloring doesn't work when some of the rows are hidden. Possibly make all groups odd and all tests even, or make the groups transparent like table header.

    The best solution may be to alternate the groups and then make all the tests white (even).

@dlangevin: If you could fix/change the one JavaScript issue that would be great.

@all: Any ideas/comments on the other two are appreciated.

dlangevin’s picture

dlangevin CreditAttribution: dlangevin commented
FileSize
simpletest_group_convention_revised_1.patch7.65 KB

OK, this patch does the three things you mentioned. It just sets the tds to 33% all the time and centers the checkboxes. I made the heading cells for each group like "odd" and everything else white. I also removed the js that expanded the table when the "select all" box was checked

dmitrig01’s picture

dmitrig01 CreditAttribution: dmitrig01 commented

You know what would be cool?
If we could label (using getInfo) each function separately.
Currently, it's per-class, which really sucks, and is not descriptive. Also, you can only run a whole class at once. Maybe we can do better? :)

boombatower’s picture

boombatower CreditAttribution: boombatower commented

That is a possibility, but I don't see much point in running only a single test function. Rarely do I even run single classes, but when writing tests. If you are writing a test and want to only run a single method it is easily done by appending a letter such as "a" to the front of the other test methods.

To confirm that something hasn't broken a specific area of code the entire suite for that area should be run, or better yet the entire set of tests as testing.drupal.org will do.

boombatower’s picture

boombatower CreditAttribution: boombatower commented
Status: Needs work » Needs review
FileSize
simpletest_group_convention_revised.patch7.7 KB

@dlangevin: Once again great work and thanks for the speedy patch.

I have noticed that the form reverts when tests are actually selected and run. This obviously needs to be fixed.

I have attached a slight modification of the previous patch.

boombatower’s picture

boombatower CreditAttribution: boombatower commented
FileSize
simpletest_group_convention_revised.patch8.26 KB

This one sorts the test groups alphabetically!!

Reminder: We also need tabs for "Unit Testing" and "Functional Testing"

boombatower’s picture

boombatower CreditAttribution: boombatower commented
FileSize
simpletest_grouping.png21.78 KB

Here is a picture of the Aggregator Tests group expanded.

boombatower’s picture

boombatower CreditAttribution: boombatower commented

Committed.

We still need to make tabs and possibly decide on how we want the groups to be named.

For instance they all end with "Tests" we could remove that and simply name the group after the module.

boombatower’s picture

boombatower CreditAttribution: boombatower commented
FileSize
split_tests_by_type.patch3.52 KB

This patch adds tabs for types of tests.

  • All
  • Functional
  • Unit

The next patch will remove Tests from ground names.

boombatower’s picture

boombatower CreditAttribution: boombatower commented
FileSize
group_naming_convention.patch22.86 KB

Cleaned up group naming convention.

boombatower’s picture

boombatower CreditAttribution: boombatower commented
Status: Needs review » Fixed

This issue has been completed.

Anonymous’s picture

Anonymous (not verified) CreditAttribution: Anonymous commented
Status: Fixed » Closed (fixed)

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