Early Bird Registration for DrupalCon Portland 2024 is open! Register by 23:59 PST on 31 March 2024, to get $100 off your ticket.
I'd like to add an interface directly to the testbot which would allow testing of a specific branch or patch.
This would radically improve the debugging process, and would also allow individuals to easily deploy a testbot just for local use.
Comment | File | Size | Author |
---|---|---|---|
#17 | pifr.local_tests_missing_defines.patch | 996 bytes | rfay |
#14 | pifr.enable-manual-tests-1176982-13.patch | 31.55 KB | jthorson |
#5 | pifr.enable-manual-local-tests-1176982-5.patch | 29.94 KB | jthorson |
#4 | pifr.enable-manual-local-tests-1176982-4.patch | 35.89 KB | jthorson |
#3 | pifr.enable-coder-manual-test-sample-1176982-3.patch | 26.41 KB | jthorson |
Comments
Comment #1
jthorson CreditAttribution: jthorson commentedOn it ... Plugin selection and Coder screenshots attached.
Comment #2
rfayNeato!
Comment #3
jthorson CreditAttribution: jthorson commentedFinished off the Coder functionality ... other than the part where we make the display all pretty instead of a var_dump.
The attached patch should work against 6.x-2.x, and also includes all of my currently pending patches from the following issues (many of are required to get local tests working in the first place):
#1214774: simpletest test_info_parse() broken for local tests
#1192680: Testbot fills up /tmpfs partition
#1217066: cron run kills run-tests.sh when running testbot tests
#1217800: 'Pass by reference' E_STRICT PHP warning in client.inc
#1214818: Testbot local 'quick' test actually runs full suite
#1207076: Improve pifr_client_db_interface 'query' handling
#1206984: Undefined function 'check_only' in /client/review/vcs/git.inc
#1205236: pifr_client_review_pifr_coder class doesn't obey $test['arguments']['coder.reviews']
#1205878: Add ability to override (currently hardcoded) Coder Severity level in pifr_coder.client.inc
Result is not polished, but there's enough there to actually use the coder functionality!
Comment #4
jthorson CreditAttribution: jthorson commentedHere's another update, with simpletest tests.
Includes all the same patches as the previous post. Given the form can handle core tests, the patch also removes the now-unused canned tests from the pifr_client.review.inc file as well.
Comment #5
jthorson CreditAttribution: jthorson commentedRerolled patch against current 6.x-2.x (which already contains the patches listed above).
This version contains both coder and simpletest manual testing functionality (which is what I meant by simpletest tests above ... I haven't developed any actual tests).
Comment #6
rfayThanks! Are there any risks to existing functionality in this one?
Comment #7
jthorson CreditAttribution: jthorson commentedI don't believe this should break any existing (working) functionality, but there could be potential risks ... it's a pretty big patch, replaces the existing canned local tests, and does touch pifr_drupal.client.inc.
Essentially, the existing pifr_drupal.client.inc had what looked like a bunch of simpletest-specific code, which I wrapped in a if(plugin=simpletest) condition in order to support coder ... if the code is ever called without this plugin variable set, it would result in most of the pifr_drupal.client.inc code not being evaluated, thus breaking the simpletest test.
Without a full PIFT+PIFR test environment, I wasn't able to fully test to ensure things were still working end-to-end ... my gut feeling is that it's fine, but I could be overlooking some bit of PIFT+PIFR logic that I'm not aware of yet.
Comment #8
rfayChatting with Jeff_S in #drupal-infrastructure:
So jthorson, boombatower, that's the ticket. Sign up those two places, and email support@osuosl.org pointing out the tickets. We'll get you in there somehow!
Comment #9
jthorson CreditAttribution: jthorson commentedSupercell access all set!
Comment #10
rfayYay - I'll get your key into the testbots then. If you could send me your preferred key (or if you want all 3 of your d.o keys that's OK too)
Comment #11
rfayI didn't realize you'd already rerolled this. We should probably go ahead and get it in. I'd like to try it out, but since you've already worked with it extensively and think the risks are minimal, I'd say go ahead and commit.
Comment #12
jthorson CreditAttribution: jthorson commentedNot quite ready ... new version coming today.
Comment #13
jthorson CreditAttribution: jthorson commentedComment #14
jthorson CreditAttribution: jthorson commentedThis one fixes the file/patch simpletest tests (relative to the patch in #5).
Comment #15
jthorson CreditAttribution: jthorson commentedCommitted to 6.x-2.x (1ceedf6) so that we may include in scratchtestbot testing.
Comment #16
jthorson CreditAttribution: jthorson commentedComment #17
rfayFirst issue - some missing defines. Did something get committed that wasn't the same as what was being tested? These defines were "off".
Comment #18
rfayThen, with that in place, I get this set of #fails when doing a local test with
7.x, role_change_notify, 7.x-1.x
Comment #19
jthorson CreditAttribution: jthorson commentedDoh ... last change I made was the name of those defines ... tested patch on testbot, copied to local machine, applied *previous* broken patch, and committed. :( Then went to test on scratchtestbot, realized I didn't have write/pull permissions, and got distracted with that.
As for the mysql issues ... the manual tests changes didn't touch anything that would cause these. Could possibly be related to #1207076: Improve pifr_client_db_interface 'query' handling (though I don't see how); but haven't seen anything like this in a week of running the manual tests. I'll re-run your tests on my local, and see if I can reproduce.
Comment #20
rfay@jthorson, if you want to be able to commit from a remote environment, use
ssh -a
to get into it.Thanks for all your work on this.
Comment #21
jthorson CreditAttribution: jthorson commentedCommitted issue in #17 (9f3328e), with a slight tweak (changed the definition instead of the reference). Believe 6.x-2.x should be functional again.
Comment #22
jthorson CreditAttribution: jthorson commentedComment #23
rfayI don't see this any more.