Tests broken since new permission in drupal core

Hi,

This patch make sense indeed.

Shouldn't we also update the info file of the module if we do this change ?

Thanks!

This patch is, according to me, valid.

I'm struggling to fix the tests, when this patch is included, I'm down to 13 fails only. (300 passes, 13 fails, 0 exceptions, and 103 debug messages)

A little help would be very welcome!

@Pol,

I tested this patch with Drupal 7.52, 7.53 and the dev version against the latest title dev version. I get a full green.
Could you please confirm that you still have problems?

Otherwise, could we try to close this ticket as soon as possible because other patches on other issues get failed tests for no reason directly related to their scope; what slow down the acceptance process.

Hi,

I think this ticket should be committed ASAP to the module.
I wrote the maintainer to see how we could speed up things.

I'll let you know when I get a reply.

Thanks!

I confirm this patch works and I also had the confirmation from @Pol.
Here are the versions we used:
Drupal: 7.53
Title: 7.x-1.0-alpha8+0-dev
Entity: 7.x-1.8
Entity translation: 7.x-1.0-beta5

Before the patch:
*281* passes, *30* fails, 0 exceptions, and 98 debug messages

After the patch:
*311* passes, *0* fails, 0 exceptions, and 104 debug messages

So we confirm the RTBC. Thanks @Stevel and @jyraya!

I will raise the Priority to Critical, because this test is preventing another from being solved:
https://www.drupal.org/node/2757739 .

As per https://www.drupal.org/node/45111 this is definitely not critical but agree that it should be committed. +1RTBC from here.

This patch makes all tests green for me. It includes the patch in #2 as well.

Note that running the tests on a drupal install with an 'en' English url prefix breaks tests due to #2724773: Translation tests fail on site with english language prefix (global $language_url is not reset in tests).

Hi,

Why these changes, tests are green with the patch from #2, what are the changes added in patch #10 ?

Thanks!

I'm testing with latest HEAD from drupal core, entity_translation and title and I get 13 fails with the patch in #2. The patch in #10 makes everything green.

@Pol: you said you got 13 fails as well, how did you fix that?

Hi,

Ok thanks for the clarification, I hope Plach will commit this asap.

Regarding the 13 fails, they are gone, check messages in #2757739: Token value is not sanitized, when replaced from title field.

Thanks for your contributions, guys!

I'm seeing a few changes in the latest patch that are a bit concerning as they may imply an undesired behavior change. I'm going to commit the attached patch, that is making tests green here in my local environment. It's basically a few hunks from #10 plus some cosmetic / coding standard fixes. I'd suggest to review only https://www.drupal.org/files/issues/title-test_fixes-2813673-15.review.txt for the actual changes.

Let's see whether the bot is happy now. We can push more changes if something is still broken.

Unfortunately it seems we have DrupalCI issues atm, so I cannot tell whether this is fixed:

https://dispatcher.drupalci.org/job/default/298540/console

The patches here will no longer apply, so failures are expected, however not even branch tests are completing atm :(

Now locally I get this test result for HEAD:

Admin settings 16 passes, 25 fails, 0 exceptions, and 5 debug messages
Field replacement 87 passes, 83 fails, 0 exceptions, and 37 debug messages
Replaced fields translation 38 passes, 67 fails, 19 exceptions, and 13 debug messages

With Entity Translation:HEAD ?

@Pol Still everything on HEAD, yes. But found out that on the web UI tests pass, on CLI they fail.

Could you try again with this patch applied too ? #2724773: Translation tests fail on site with english language prefix (global $language_url is not reset in tests)

I have all green with the latest -dev versions of Drupal core, ET, Entity and Title. Both via run-tests.sh and UI.

Let's wait for the bot to see whether at least he's happy.

I suspect that due to the way tests are written right now, they may somehow be affected by the host installation.

@Pol Yes, I was using the patch from #2724773: Translation tests fail on site with english language prefix (global $language_url is not reset in tests).

mmmh... running out of idea :(

Tests are green here: https://drive.google.com/file/d/0B7ugUjBv1lqHc05teHk1NFhfNUU/view?usp=dr...

Actually branch tests are failing but with different numbers than #19. From looking at the errors I suspect this has something to do with the installation profile. Let's try the attached patch.

for possible hints to fix this for the title module tests, see
entity_translation git diff 0cd07cf4769a92 a8ae1e78c57c6

diff --git a/tests/entity_translation.test b/tests/entity_translation.test
index 2a647be..b552ed1 100644
--- a/tests/entity_translation.test
+++ b/tests/entity_translation.test
@@ -64,6 +64,7 @@ class EntityTranslationTestCase extends DrupalWebTestCase {
       $this->admin_user = $this->drupalCreateUser(array_merge(array(
         'bypass node access',
         'administer nodes',
+        'administer fields',
         'administer languages',
         'administer content types',
         'administer blocks',

Minor changes to the setUp() methods.

Oddly enough, all of the tests pass locally, but they still fail on drupalci trying to enable the modules.

Try changing:

$admin_user = $this->drupalCreateUser(array('administer site configuration', 'administer taxonomy'));

to:

$admin_user = $this->drupalCreateUser(array('administer site configuration', 'administer taxonomy', 'administer fields'));

The fact that so many of the setUp() methods are failing when just enabling the modules is kind of messed up :-\ I need to check the console output.

I wonder if we need to get the test dependencies committed first, then work on the tests themselves?

Lets split the tests into pieces so they're individually easier to maintain. Also, lets list the dependencies required for the tests.

The tests still pass locally.

I don't suppose a maintainer could test this locally and then commit it? :) I'm happy to help get this issue finished but I think we've hit an infrastructure problem.

This updates getInfo() with the dependencies lines, where appropriate.

not sure why my patch is so much bigger than 37..

new patch

ok, I think this might help.

in 2016 I fixed similar issue in webform_localization by re-ordering the modules in setUp.

taxonomy should come last so that it actually gets installed first.

Still the same number of errors. Putting the cleaner code back.

Same results, as expected.

Lets go back to the step we know we need - this adds the test dependencies to the info file.

Duh, don't need to list taxonomy as a test dependency X-)

One other small tweak - there's no need to list title.module as a files[] line as there are no classes defined in it.

taxonomy , why?

There's a module called https://www.drupal.org/project/taxonomy_access_fix

I'm working on getting the drupalci test suite to run locally so I can confirm whether committing #56 will solve the initial problems.

+    /* Reset the permissions cache prior to calling drupalCreateUser
+     * see notes here: https://api.drupal.org/comment/28739#comment-28739
+     */
+    $this->checkPermissions(array(), TRUE);

Do this, will probably help the permissions issues. do this right before drupalCreateUser

I really don't think that matters, given that the tests work a-ok locally.

Just to be clear - it doesn't matter what we do with permissions, etc if the modules aren't being enabled correctly, that's the source of the problems.

Committed and pushed #56, thanks for your efforts here, hugely appreciated!

Whoo-hoo, tests are passing again! Awesome work!

https://www.drupal.org/node/11063/qa

Thanks. I'd still like to get the other test refactoring included, so here's that patch.

All green, now that's more like it :)

TitleTranslationTestCase.test is missing from the patch.

Aside from that, is there any actual change to the test code? If not can we have a simple rename patch? You may need to add the following lines to your git config:

[diff]
  renames = copies

Whoops! Thanks for noticing the missing file.

This patch makes a few changes:

• Each class is split into its own file.
• The test dependencies are listed in getInfo().
• The $profile variable is overridden properly.
• The setUp() method's prototype is updated to the proper format.
• The setUp() method's modules listing is streamlined.

+++ b/tests/TitleAdminSettingsTestCase.test
@@ -0,0 +1,80 @@
+    $modules[] = 'locale';
...
+    // Other dependencies.
+    $modules[] = 'entity';
+    $modules[] = 'entity_translation';

Why are we adding these dependencies?

Good question. I'm not sure =) Must have been a copy/paste mistake.

This removes the unnecessary dependencies from that one test.

Committed and pushed, thanks!

Woohoo, thanks!