Comments

Niklas Fiekas’s picture

Assigned: Unassigned » Niklas Fiekas

Doing this next.

Niklas Fiekas’s picture

Assigned: Niklas Fiekas » Unassigned
Status: Active » Needs review
StatusFileSize
new101.15 KB
new50.66 KB

Classes converted.

I have also attached the concatenated output of git blame -C on the new files, showing that the code is coming mostly 1:1 from the old test file. Best viewed without Dreditor or automatic linewrapping, or zoomed out, to get an overview.

Status: Needs review » Needs work

The last submitted patch, 1598558-cache-tests-psr-0-2.patch, failed testing.

Niklas Fiekas’s picture

Status: Needs work » Needs review
StatusFileSize
new722 bytes
new84.15 KB

Had to prefix one instance of \stdClass.

Niklas Fiekas’s picture

Had to prefix on instance of stdClass with \.

Huh ... wierd. When I tried to save the comment it said "You have no access to any project." Then, visiting this issue, no comment appeared. So posting the same thing again. And now both are there ;)

aspilicious’s picture

Status: Needs review » Needs work

Can we put use stdClass on top of the file (like we have done in other place in core) for the moment. Untill we decide to undo that ;)

Niklas Fiekas’s picture

Status: Needs work » Needs review
StatusFileSize
new873 bytes
new50.65 KB

Oh, yes, we can. Actually I wanted to do this right after opening the discussion issue, to not hold this up. Thanks for the reminder. Also solved a merge conflict in the .info file.

(And it looks like there were reasons to have use-statements rather than inline slashes, although I don't like that.)

aspilicious’s picture

Status: Needs review » Reviewed & tested by the community

good!

Niklas Fiekas’s picture

Issue tags: -PSR-0

I believe this doesn't apply anymore. #7: 1598558-cache-tests-psr-0-7.patch queued for re-testing.

Status: Reviewed & tested by the community » Needs work
Issue tags: +PSR-0

The last submitted patch, 1598558-cache-tests-psr-0-7.patch, failed testing.

Niklas Fiekas’s picture

Status: Needs work » Needs review
StatusFileSize
new3.87 KB
new50.7 KB

Using the chance to also rename TestBase to the less generic CacheTestBase, even though the namespace is already Cache, as discussed in IRC.

aspilicious’s picture

Status: Needs review » Reviewed & tested by the community

Great, we don't need to reroll if the .info file conflicts. Catch is going to ignore that and leave that cleanup for the end.

jhodgdon’s picture

In that case, I'll leave the rest of these for catch to commit, since I don't know anything about this plan.

robloach’s picture

Issue tags: -PSR-0

#11: 1598558-cache-tests-psr-0-10.patch queued for re-testing.

Status: Reviewed & tested by the community » Needs work
Issue tags: +PSR-0

The last submitted patch, 1598558-cache-tests-psr-0-10.patch, failed testing.

robloach’s picture

Status: Needs work » Needs review
StatusFileSize
new31.72 KB

Re-roll without the system.info change, with rename detection.

aspilicious’s picture

Status: Needs review » Reviewed & tested by the community

back to rtbc

aspilicious’s picture

Status: Reviewed & tested by the community » Needs work
Niklas Fiekas’s picture

Status: Needs work » Needs review
StatusFileSize
new44.88 KB

Merged.

robloach’s picture

Status: Needs review » Reviewed & tested by the community

testMinimumCacheLifetime, setupLifetime, and the section of CacheInstallTestCase arn't presented in the patch anymore. Looks good!

webchick’s picture

Status: Reviewed & tested by the community » Fixed

Committed and pushed to 8.x. Thanks!

aspilicious’s picture

Status: Fixed » Needs work

You accidently forgot to add the new files.

aspilicious’s picture

Status: Needs work » Reviewed & tested by the community
aspilicious’s picture

StatusFileSize
new24.35 KB

Should do it, lets wait for the bot

Niklas Fiekas’s picture

Priority: Normal » Major

RTBC seconded.

webchick’s picture

Status: Reviewed & tested by the community » Fixed

Now with git apply --index. :P

Committed and pushed to 8.x. Thanks!

Niklas Fiekas’s picture

Priority: Major » Normal

Yay! Thanks.

Niklas Fiekas’s picture

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