Tests are currently failing on Drupal.org, which is causing any submitted patch to be rejected. The problem occurs when the system running the test doesn't have the module itself enabled. The reason for this is that the test linkit_profile.test (the one that is failing) is using the constant LINKIT_PROFILE_TYPE_EDITOR from the main module file to set a value in the add profile admin screen. This constant is undefined when the module is not enabled, like on d.o.
I'll attach a patch shortly.
| Comment | File | Size | Author |
|---|---|---|---|
| #8 | linkit-fix-profile-test-2443487-8.patch | 699 bytes | eric_a |
Comments
Comment #1
eelkeblokHere's the patch I promised. It also contains the addition of a newline at the end of the file, because my phpStorm is configured to add them automatically. I hope you'll forgive me (FWIW, it is a small coding guideline violation).
Comment #2
eric_a commentedThe test tries to enable modules, but won't succeed if external dependencies aren't being build. See attached patch. Note that after committing such a fix, the testbot will build these dependencies only after the dependency chain has been rebuilt, which is only after the next snapshot has been built, if I remember correctly.
Comment #3
eric_a commentedOops. Implementing getInfo() in the abstract base class by itself will not fix the child class implementations...
Comment #4
eric_a commentedHmm, the current 7.x-3.x seems to be broken, but in #2405093: Updating from 2.x to >=3.3 fails with PDOException the patch from #8 was tested against 7.x-3.x and external dependencies ctools and entity *were* checked out by testbot. Perhaps the fact that in that issue the version is set to 7.x-3.3 has something to do with this.
Comment #5
eelkeblokFWIW, with the patch in comment 1 everything worked fine with Linkit disabled on the testing client, on the latest 3.x commit.
Without the patch:
What makes you say this needs work? Have you tried running the tests locally with and without the patch?
Comment #6
eric_a commentedNo, I did not run the tests locally. Believing that the module code was not built, I went down the external dependencies path, but while that is an issue on its own, I now realize that the real issue is that the constant will never just be available at test compile time. Yes, #1 fixes this, but IMHO it's wrong to define the same constant in multiple places. Including the module file will help, as would using the constant later in the test flow, at a point when the module is enabled.
Setting to needs review for #1.
Comment #7
eric_a commentedComment #8
eric_a commentedHere's putting the constant to use when it is defined.
Comment #9
eelkeblokThat's indeed a better solution.
Comment #10
anonThanks for the patches. This is now fixed.
Comment #15
anonClosing again, the test bot change the status for some reason.
Comment #16
ashalan commentedTests are failing on d.o because of this issue. The test are correctly failing because of faulty data in the db.
Comment #17
eric_a commentedWell, the reason that #8 fails is that now that the branch testing is green the postponed testing is resumed, but since this patch is in already it obviously won't apply anymore.
Comment #18
eric_a commentedOh, I just saw the note on the project page. This project had a lot of postponed bot tests. They all ran now because of the branch being fixed. But since the patches are committed already the tests fail now. So basically: don't commit patches when the branch test has failed. Except for the commit that fixes the branch.