Closed (fixed)
Project:
Drupal core
Version:
8.1.x-dev
Component:
file system
Priority:
Normal
Category:
Bug report
Assigned:
Unassigned
Reporter:
Created:
15 Jul 2016 at 19:21 UTC
Updated:
17 Aug 2016 at 00:04 UTC
Jump to comment: Most recent, Most recent file
Comments
Comment #3
claudiu.cristeaComment #4
claudiu.cristeaComment #6
klausiMakes sense!
Fixing title for gender neutrality.
why is this a kernel test? No need for a database here, so this can be a unit test, right?
assertFalse() is not correct here, since the result of the bitwise AND will be another integer (type mismatch). So this should be assertSame(0, $type & StreamWrapperInterface::WRITE)
Comment #7
claudiu.cristea@klausi, thank you
Comment #10
claudiu.cristeaComment #13
claudiu.cristeaI don't understand this CI error :(
Comment #15
MixologicSorry I didnt see the ask in #drupal-contribute, but I ran the testbot locally, and its a pretty classic error. The testbots should be better about this kind of reporting:
Comment #16
claudiu.cristea@Mixologic,
Ah, right, stupid me. PhpStorm has auto created @package instead of @group and I didn't check twice.
Comment #17
claudiu.cristeaComment #19
claudiu.cristeaIn #16 we have the prove. Now, the patch.
Comment #21
claudiu.cristeaWhat?
Comment #23
claudiu.cristeaThe failure is not related to this patch. See #2749955: Random fails in UpdatePathTestBase tests.
Comment #24
amateescu commentedCan't we add this assertion to the existing
\Drupal\KernelTests\Core\File\ReadOnlyStreamWrapperTest::testReadOnlyBehavior()instead of creating a whole new test class just for a method?Comment #25
claudiu.cristea@amateescu, yes, that makes a lot of sense.
In 8.1.x
ReadOnlyStreamWrapperTestwas an old-style kernel test and was placed inside system.module, thus I provided also a patch for 8.2.x.Comment #26
amateescu commentedAwesome :)
Comment #28
claudiu.cristeaThis is #2749955: Random fails in UpdatePathTestBase tests. Back to RTBC as per #26.
Comment #29
alexpottCommitted 7b0d229 and pushed to 8.2.x. Thanks!
Committed 48b3d70 and pushed to 8.1.x. Thanks!
Comment #32
claudiu.cristeaUnfortunately I forgot to add also the
StreamWrapperInterface::LOCALbit to the result ofLocalReadOnlyStream::getType(). I was wrong, thinking that READ_VISIBLE composite value contains also LOCAL without testing that. I opened a followup in #2770339: LocalReadOnlyStream must declare itself LOCAL.