NodeAccessLanguageTest has a call to variable_set for node_access_test_secret_catalan that is incorrect as the module that uses this has been converted to state. Also that is no test that actually explicitly tests the effect of node_access_test_secret_catalan. One issue with the tests is that we don't check for the opposite condition.
Making a major as I'm removing a variable set.
Comment | File | Size | Author |
---|---|---|---|
#5 | 2175739.5.patch | 9.13 KB | alexpott |
#5 | 1-5-interdiff.txt | 1.2 KB | alexpott |
#1 | 2175739.1.patch | 8.88 KB | alexpott |
#1 | 2175739.1.will-fail.patch | 8.81 KB | alexpott |
#1 | interdiff.txt | 800 bytes | alexpott |
Comments
Comment #1
alexpottThis test is had to read because it uses bad variable names and creates things it never actually tests on. For example in
testNodeAccessPrivate
it creates $node that it never actually uses and it calls$node_public = $this->drupalCreateNode(array('body' => array(array()), 'langcode' => 'hu', 'private' => TRUE));
- the whole point is that this node is not actually public :)The patch attached:
drupal_static_reset('node_access');
this is just incorrectnode_access_test_secret_catalan
A "will fail" version is also attached where all the refactoring to the tests is still there but the old variable_set remains. The interdiff is between the two patches.
This patch is easier to review by applying and reading the test rather than looking at the patch.
Comment #3
ianthomas_ukComment #4
BerdirLooks all fine to me, tried it locally and the resetCache() call didn't seem to have an effect, it passed without that too, in both cases.
Just wondering if that's a problem, or if we're just doing it because it would otherwise also pass without being tested.
Not sure if it would make sense or if it's even possible to have a call that would fail when the static cache is still in place?
Comment #5
alexpottAdded a test to show that the static cache can be tested and why it needs to be cleared.
Comment #6
alexpottThis should actually be critical since it is blocking #2167109: Remove Variable subsystem
Comment #7
Gábor Hojtsy@Berdir: looks good? :)
Comment #8
BerdirYes, RTBC if testbot agrees.
Comment #9
webchickCommitted and pushed to 8.x. ROCK!