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.

Comments

eelkeblok’s picture

Priority: Normal » Major
Status: Active » Needs review
StatusFileSize
new739 bytes

Here'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).

eric_a’s picture

StatusFileSize
new611 bytes

The 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.

eric_a’s picture

Status: Needs review » Needs work

Oops. Implementing getInfo() in the abstract base class by itself will not fix the child class implementations...

eric_a’s picture

Hmm, 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.

eelkeblok’s picture

FWIW, 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:

  • I would get the exact same error as d.o is throwing about not being able to set the profile type to LINKIT_PROFILE_TYPE_EDITOR when Linkit was not enabled on the client system, e.g. the test would now know the value for LINKIT_PROFILE_TYPE_EDITOR and it would just treat the constant as a string value, as PHP does.
  • It would work fine if I enabled Linkit, because the constant would be known then. That's probably the situation for most people working on this module, yet not for drupal.org/the testbots).

What makes you say this needs work? Have you tried running the tests locally with and without the patch?

eric_a’s picture

Status: Needs work » Needs review

What makes you say this needs work? Have you tried running the tests locally with and without the patch?

No, 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.

eric_a’s picture

eric_a’s picture

StatusFileSize
new699 bytes

Here's putting the constant to use when it is defined.

eelkeblok’s picture

Status: Needs review » Reviewed & tested by the community

That's indeed a better solution.

anon’s picture

Status: Reviewed & tested by the community » Fixed

Thanks for the patches. This is now fixed.

  • anon committed 8dba51e on 7.x-3.x authored by Eric_A
    Issue #2443487 by Eric_A, eelkeblok: Tests are failing for 3.x on d.o
    

The last submitted patch, 1: linkit-fix-profile-test-2443487-1.patch, failed testing.

The last submitted patch, 2: linkit-external-dependencies-2443487-2.patch, failed testing.

Status: Fixed » Needs work

The last submitted patch, 8: linkit-fix-profile-test-2443487-8.patch, failed testing.

anon’s picture

Status: Needs work » Fixed

Closing again, the test bot change the status for some reason.

ashalan’s picture

Tests are failing on d.o because of this issue. The test are correctly failing because of faulty data in the db.

eric_a’s picture

Well, 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.

eric_a’s picture

Oh, 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.

Status: Fixed » Closed (fixed)

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