As it is now (after #1007210: Test bot should run tests against the core version the issue is set to. was fixed), patches posted in an issue are tested against the version the issue is set to. There are times though when patches for more than one branch/version are posted at the same time. Only one of them is actually tested leaving the other(s) simply ignored, because issues -and thus patches- can only be "tied" to a single version at a given time.
Now that #66484: Allow issues to be filed against multiple versions/branches. is in the works, we should make it possible to assign a version to an uploaded file if that is a patch. Then each patch can be tested against its version respectively.
As to how this can be achieved, for now we can go the easy(ier) way of having a filename based rule. In the long run, perhaps we'll make it possible to be able to select version while uploading the patch file.
To do
- Show version tested against in the test bubbles.
- Allow a patch to be tested against multiple versions via custom tests.
- Add a UI for selecting versions for testing to the Files fieldset. See also #1162082: Add a warning if a diff/patch file is selected and issue status is not testable
Comment | File | Size | Author |
---|
Comments
Comment #1
klonos...we already have a rule in place to ignore patches that have a filename that ends in "-do-not-test.patch". Some are already in the (good) habit of adding a "d7" or "d8" in their patches. Why don't we use that?
Comment #2
jthorson CreditAttribution: jthorson commentedMoving over from abandoned testing infra queue.
Comment #3
klonosThanx. I really hope we could implement this after D7 d.o launch.
Comment #4
klonosComment #5
helmo CreditAttribution: helmo commented+1 for making the test bot smart enough to recognise d7 and d8 suffixes.
Comment #6
isntall CreditAttribution: isntall at Drupal Association commentedComment #7
xjmComment #8
xjmThis is going to be very important as we begin to manage development in several minor branches simultaneously.
Comment #9
xjmTo some extent, #2462101: [policy, no patch] Update backport policy to account for 8.x minor versions and 7.x divergence will help work around this problem for major branches, although we still need to address the issue for minor versions. And #2268449: Run contrib module branch tests against both dev and latest stable core branches has addressed a similar problem for contrib project testing, but we need something similar for patch testing for core at least.
I think a first incremental step is to at least mark on an issue which branch a patch has been tested against, which is not clear currently. The QA pages for projects (e.g. https://www.drupal.org/node/3060/qa) show this information now, but patches on issue do not. (That might need a separate issue filed; I'll make a note on #2559711: [meta] Drupal.org (websites/infra) blockers to Drupal 8.0.0, 8.1.0, etc..)
Comment #10
Mile23This is also a problem in contrib, where all 8.x patches always test against 8.1.x.
Comment #11
xjm@Mile23, that sounds like "works as designed" or maybe something with the particular project's QA configuration; maybe open a separate bug report since that is not related to this? (Edit: since following the other issue above that's supposed to be configurable now for contrib.)
Comment #12
Mile23The clue is in that link: https://www.drupal.org/node/3060/qa
drupalci_api and thus the runner can check out whatever branch you tell it to, so d.o needs to tell it the right thing in the request, based on its UI. So it has to change the UI to reflect all branches.
Comment #13
drummmile23 - #2651208: More-flexible, while reasonable, configured tests is the issue for that. You can currently kick off a custom test by clicking add test and select any core branch.
I think we should have a UI for this. We have some filename conventions like
*-d6.diff
; that could prepopulate the UI, but wouldn't be a friendly UI on its own.That means files could be replaced with a field collection (or equivalent) of the file and a version. Not a fun migration at all.
That leaves putting a multivalued version field next to the file field, syncing up the fields' deltas and making the UI generally work. Still not ideal, but there wouldn't be a migration needed.
(I haven't looked at the field collection alternative lately, maybe another contrib module would be more convenient?)
Comment #14
Mile23I just made this issue: #2663018: Useful core branch defaults for contrib CI
Filename conventions aren't so bad but are also terrible. :-)
Branch information can be derived from .info.yml, and also test definition files, either of which could be modified by patches. I think it's fair to say that a patched test definition file will override d.o's UI in drupalci_api, unless we also add a way to override that.
I think @jthorson is correct over here #2655718: Create the BuildResults in that we need to have a way to communicate back to results/d.o which branch was checked out, so it can be displayed to the user. This falls into scope for three projects: drupalci_results, drupalci_testbot, and d.o.
It's a matter of choosing where to start, because it seems development resources are thin. The point of entry with the most commonality seems to be the JobResults encapsulation and support structure, and figuring out how to show it to the user.
Comment #15
jthorson CreditAttribution: jthorson commentedI think that sorting out the UI for associating versions with individual files can definitely happen in parallel ... full integration with the testing platform is dependent on sorting out required changes to the DrupalCI / Drupal.org integration; but having the ability to indicate what version a particular patch was built for has some inherent value on it's own, even without full integration with the testing stack.
Implementation is a bit tough, for the reasons that drumm outlined above ... I think migration to a full field collection is probably a non-starter.
The 'quick and dirty' approach would be a new module (or additions to the drupal.org module) which takes advantage of the various Extended File Field hooks:
I think looking at how PIFT uses these hooks to associate additional information with each file on an issue might provide a good start; as adding per-patch test results without changing the underlying issue_files field isn't all that far off of adding per-patch version associations.
Comment #16
jthorson CreditAttribution: jthorson commentedRe: #14 - for the moment, we can disregard the drupalci_results project ... the intent there was to provide code for a standalone Results server that would be linked to from Drupal.org; but the direction has changed to storing those results on Drupal.org directly.
Comment #17
drummFor core 8.0.x and 8.1.x, and other projects with similar branches, we should try making this multi-select.
Comment #18
drummThinking about this a bit more, I think PIFT should do this on its own. The pift_ci_job object has a
release_nid
property that keeps track of the release branch being tested. We can use the extended file field hooks to make a UI for this, and replacepift_test_add_files()
's call topift_ci_find_dynamic_release()
.Comment #19
drummI'll be taking this issue on this week. I've added a todo list to the issue summary.
Since we do want a single file to be able to be tested against multiple versions (like 8.0.x and 8.1.x and other similar branches), we should add this to the UI, probably within the test bubbles. This information has already been added to the test pages with #2664370: Add branch tested with to issue test pages.
The UI on the Files fieldset will have the side effect of solving #1162082: Add a warning if a diff/patch file is selected and issue status is not testable. If there isn't a place to choose which version to test against, we can at least show a little message saying it won't be tested. I'm keeping that open as a separate issue, it should be a good incremental improvement.
Comment #20
drummThis is the direction I'm going for the test display.
I decided to group the tests by the project version to keep results organized and less repeating everything in each bubble. I hid the column that always says "new" to make a bit more room.
I have queueing these up working with the custom tests UI, after clicking add test. I may be able to get that deployed as soon as today. The UI in the Files fieldset is still needed.
Comment #23
drummHere is where I've gotten the file upload UI:
This still needs some testing, I expect it can be deployed tomorrow if that goes well.
Comment #24
catchThis screenshots look pretty good and this would help with the 2-3 core branch issue a lot.
Comment #26
drummThanks, I think I have this tested well enough to be ready now.
Comment #27
drummThis has been deployed.