Early Bird Registration for DrupalCon Portland 2024 is open! Register by 23:59 PST on 31 March 2024, to get $100 off your ticket.
Although #1551248: Port project-specific "Assigned" functionality to D7 is now fixed, I realized that we somewhere lost the tests for the issue maintainer functionality that we used to have in D6. It'd be nice to port all these to D7.
Level Of Effort
It is estimated to take less than one day to complete this task.
Comment | File | Size | Author |
---|---|---|---|
#22 | 0001-Issue-1633020-follow-up-Test-s-setUp-method-should-b.patch | 1.94 KB | marvil07 |
#13 | 1633020-13.patch | 5.34 KB | kalman.hosszu |
Comments
Comment #1
Senpai CreditAttribution: Senpai commentedComment #2
paulovelho CreditAttribution: paulovelho commentedThe test was having some troubles to find the options in the select box.
I had to change the options sent to the options that were being displayed in the web page. The name of the select box was different from the expected as well.
I'm not quite sure how the options and names are being created, but at least the tests are successful now.
There is still some notices in the test result. Like this one:
Undefined property: ProjectIssue_SelectionHandler_Assigned::$entity Notice ProjectIssue_SelectionHandler_Assigned.class.php 59 ProjectIssue_SelectionHandler_Assigned->getAllowedAssignees()
Comment #3
paulovelho CreditAttribution: paulovelho commentedComment #4
Nick_vhThanks Paulo! A patch in your first two hours in the Drupal world! Well done!
Hurray for the Drupal.org upgrade sprint. Go London team ;-)
Can someone confirm if this can be improved so we do not have to define the [und]?
Can someone tell us what this placeholder is?
Comment #5
bdragon CreditAttribution: bdragon commentedThe placeholder is the language. LANGUAGE_NONE is 'und', you will see it all the time when working with fields.
Comment #6
Nick_vhI was referring to "placeholder-for-uid-1". Can you clarify that?
Comment #7
dwwCool, thanks for the patch and work on this! However, there are still some lingering problems here:
A) Just visiting the simpletest page, I'm seeing this warning:
B) when I run the tests, I'm getting a 16 failures, most of them like this:
I agree the 'placeholder-for-uid-1' stuff is bogus and should be removed. Similarly, 'Anonymous' shouldn't be there at all (as explained at #1551248: Port project-specific "Assigned" functionality to D7).
C) However, there are 4 failures like this:
Those are on lines 56, 61, 92, and 96 of the patched maintainers.test file. So, each of those 4 assertions needs to be investigated to see if it's a bogus test or an actual bug. ;)
Thanks,
-Derek
Comment #8
rgristroph CreditAttribution: rgristroph commentedI'm taking a look at this, assigning to myself.
--Rob
Comment #9
dwwRough estimate of 3 hours. I don't think rgristroph is actively working on this. Please re-assign to yourself if I'm wrong. ;)
Thanks,
-Derek
Comment #10
drummComment #11
drummComment #11.0
drummAdding a level of effort.
Comment #12
kalman.hosszu CreditAttribution: kalman.hosszu commentedI'm starting to check this one.
Comment #13
kalman.hosszu CreditAttribution: kalman.hosszu commentedI attached a patch. It works with my localhost but please review it.
Entityreference module generates a string for "Assigned" select element options called "- Restricted access -" so I checked the string too, if you have any better idea to check that test case, please write it to me.
Comment #15
drummThanks!
Comment #16
drummhttps://qa.drupal.org/pifr/test/268788 says:
Comment #17
kalman.hosszu CreditAttribution: kalman.hosszu commentedIt's interesting, I just ran the project tests in my localhost and it was passed "174 passes, 0 fails, 0 exceptions, and 54 debug messages"
Could you re-run the tests?
Comment #18
drummI re-ran the tests, https://qa.drupal.org/pifr/test/268788. I think it has the same failed results as I mentioned in #16.
The most details are under "Review log". It could be a tagged version of a dependency was used instead of -dev. There is also an FAQ in the sidebar.
I'd be interested to know if the tests pass on a Drupal.org dev site.
Comment #19
marvil07 CreditAttribution: marvil07 commentedThe included patch depends on #2262415: Test's setUp() method should be protected to avoid a php fatal(a tests inherits from a project test class which contains the same visibility bug).
It fixes the strict warning mentioned on comment 7.
The usual problem with both is one of the mentioned items in the FAQ. Sadly, even running tests from CLI, with the method testbot uses I could not yet reproduce the problem with my local install(even from a from-scratch site).
FTR the line I'm using:
I have decided to review the dependencies on the testbot run as drumm suggested, which ended up in some strange findings:
A. entityreference-7.x-1.0 is used instead of the current stable 7.x-1.1 (not recent, so sounds like fixed version dependency somewhere). Two assertions from ProjectIssueMaintainersTestCase fails downgrading this to testbot used version (same as testbot).
B. project version used is 7.x-2.0-unstable1, which is really old (2013-Jan-22), new release please :-). The other failing assertion, from ProjectIssueMaintainersTestCase can be reproduced downgrading to that version.
In both cases I did not look at the details yet, but it seems like it's better to just update the dependency.
For A. I do not have a solution, I could not find any info file using an explicit version dependency for entityreference. Maybe a dependency builder bug? Not sure.
For B a new release for project project is needed.
Comment #21
drummThe patch in #19 looks the same as #2262415: Test's setUp() method should be protected.
So far, #20 hasn't made a difference. I think it may take awhile to pick up on it.
Comment #22
marvil07 CreditAttribution: marvil07 commentedA and B mentioned on comment 19 are still happening(I have just reviewed the log again on https://qa.drupal.org/pifr/test/268788), so yes, we need to wait the d.o project dependencies rebuild.
Sorry about the patch, it seems I confused it while uploading. Added now a new one, please notice this will not fix the real problems but use the right visibility for the setUp() test methods.
Comment #27
drummThanks everyone! The branch tests now pass. #26 is of course the testbot seeing if it can apply the patch a second time.
#24 was the trick for getting -dev instead of unstable1 for project. The testbot rightfully wants to test against more-stable dependencies.
I don't know if #25 is actually required, but it doesn't hurt to be explicit. The actual fix was to run the project_dependency_rebuild_project Jenkins job on entityreference. For whatever reason, the project dependency data didn't know about 1.1 at all.