Summary

Currently, the 'id' column contains "file_id" for file tests, and "release_nid" for branch tests.

This proposal is to add a new 'nid' column to the {pift_test} table. After this addition, the 'nid' column would be used to store the "issue_nid" for file tests, and the "release_nid" for branch tests. The 'id' column would continue to store "file_id" for file tests, but would now store the unique "label_id" from versioncontrol_labels for branch tests (which can be used to determine both project_id and git version string).

This is step one in the quest towards supporting automated testing on sandbox projects, and should also make it possible to solve #675460: Ensure/Refactor cron_retest() query to only re-test the last file on an issue; Automatically retest RTBC patches.

Tasks

Database changes
1. Add a new 'nid' column to pift_test.
2. Update pift_test, datafilling the 'issue nid' for file tests, and copying the 'release nid' over from the 'id' column for branch tests.
3. Update the 'id' column with label_ids from versioncontrol_labels for all branch tests.

Code changes
1. When writing a new file test to pift_test, include the issue nid.
2. When writing a new branch test to pift_test, include the label_id in id, and release_nid in nid (if one exists)

NOTE: Everything below here becomes part of the next step (the introduction of label_ids); which is out of scope for this issue to keep the patch reviews manageable.

=== Out of scope for this particular issue ===============
3. When creating a 'branch' batch, check if the branch 'nid' is datafilled.

  • If so, use that as the 'client identifier' for the batch.
  • If not, check if the label has a release node.
    • If so:
      • update the 'nid' column
      • set $batch['branch'][x]['update_id'] = $label_id
      • use 'nid' as the client identifier
    • otherwise, use 'id' as the 'client identifier' for the batch.

Pros:
1. No db manipulation on PIFR required
2. Should make #675460: Ensure/Refactor cron_retest() query to only re-test the last file on an issue; Automatically retest RTBC patches trivial. (+1!)

Cons:
1. Would result in duplicate branch test records on qa.d.o, one from the pre-release test, and a new entry after the official release.

  • This is solved with a slight tweak to PIFR, having it key off the $batch['branch'][x]['update_id'] value in the $batch array, and updating the value in it's own pifr_branch table.
CommentFileSizeAuthor
#6 pift.add_nid_to_pift_test_table_6.patch16.74 KBjthorson
#5 pift.add_nid_to_pift_test_table_5.patch16.23 KBjthorson
#3 pift.add_nid_to_pift_test_table.patch15.85 KBjthorson
#1 pift.test_db_update.patch2.02 KBjthorson

Comments

jthorson’s picture

jthorson commented
StatusFileSize
new pift.test_db_update.patch2.02 KB

This is just the .install changes, which adds the column and copies the correct values into the correct columns. Without the other code changes this patch WILL BREAK THE SITE! Be warned. :)

jthorson’s picture

jthorson commented

File test entry conversion looks good.

Minor side effect on branch tests ... releases which have since been deleted end up with the release node in both id and nid columns. Not a big deal (we could probably even delete these tests, as the releases no longer exist). Just noting here, in case it causes problems later on during the development/testing.

jthorson’s picture

jthorson commented
StatusFileSize
new pift.add_nid_to_pift_test_table.patch15.85 KB

This might do it ... will test on rfay.redesign.

jthorson’s picture

jthorson commented

Well ... batch was built and sent to qa.scratch ... but then it was sent again, and again, and again ...

rfay.redesign restored to normal. Parking this for the night.

jthorson’s picture

jthorson commented
StatusFileSize
new pift.add_nid_to_pift_test_table_5.patch16.23 KB

Current patch with results of testing:

Automated Testing Tab - Delete Test:
- Delete test worked, but redirect BROKE.

Automated Testing Tab - New Test:
- PIFT Queue -> Sent transition good.
- qa.scratch Receive batch -> Queued good.
- qa.scratch Queued -> Tested good.
- qa.scratch Result -> PIFT good.

Automated Testing Tab - Re-Test:
- PIFT Queue -> Sent transition good.
- qa.scratch Receive batch -> Queued good.
- qa.scratch Queued -> Tested good.
- qa.scratch Result -> PIFT good.

Issue Queue Patch - Re-Test:
- Comment auto-followup: good
- PIFT Queue -> Sent transition good.
- qa.scratch Receive batch -> Queued good.
- qa.scratch Queued -> Tested good.
- qa.scratch Result -> PIFT good.

Issue Queue Failed Patch - Re-Test:
- Comment auto-followup: good
- PIFT Queue -> Sent transition good.
- qa.scratch Receive batch -> Queued good.
- qa.scratch Queued -> Tested good.
- qa.scratch Result -> PIFT good.
- Failure comment followup -> BROKE

jthorson’s picture

jthorson commented
StatusFileSize
new pift.add_nid_to_pift_test_table_6.patch16.74 KB

This version should fix the delete redirect, and allow me to investigate the remaining failure (qa.d.o to d.o communication on fail result).

jthorson’s picture

jthorson commented
Status: Active » Needs review

Patch looks good ... the comment followup failure was due to the 'failed testing' message only appearing for the last patch in an issue, while I was re-testing the second last.

Will leave active on rfay.redesign ... Ready for review, testing, and soak.

jthorson’s picture

jthorson commented
Version: 6.x-2.x-dev » 7.x-2.x-dev
Status: Needs review » Fixed
Issue tags: -project application revamp +drupal.org D7, +testbot

Re-done for PIFT 7.x-2.x, as part of the drupal.org D7 migration.

Committed as http://drupalcode.org/project/project_issue_file_test.git/commit/451f7e8...

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

Anonymous’s picture

(not verified) commented
Issue summary: View changes

Separating intro of nid column from intro of label_id.