Problem/Motivation
While working on #2287071: Add cacheability metadata to access checks, I had to update AccessManagerTest
. In doing so, I could not get two tests to pass.
After wasting many hours on investigating why this is the case, it turns out it's because the container was being used incorrectly.
Proposed resolution
$this->container->register('test_access', $access_check);
should have been
$this->container->set('test_access', $access_check);
in both tests.
Now the mocked access checker's (mocked) applies()
method will actually (finally) be invoked. Hurray! However, doing so reveals that the arguments resolver was not yet being mocked for both of these tests. So I added that as well.
To prevent such regressions in these tests in the future, I changed the mocked access checkers' methods from any()
to atLeastOnce()
, which would have revealed this problem immediately.
In other words: these tests have never worked!
Remaining tasks
Review + commit.
User interface changes
None.
API changes
None.
Comment | File | Size | Author |
---|---|---|---|
access_manager_test_broken.patch | 3.24 KB | Wim Leers | |
Comments
Comment #1
dawehnerWe should certainly not use $this->any() anymore, unless not possible. ... #2222719-16: Use parameter matching via reflection for access checks instead of pulling variables from request attributes
Comment #2
alexpottCommitted bf7b1b0 and pushed to 8.x. Thanks!