Support from Acquia helps fund testing for Drupal Acquia logo

Comments

cecrs’s picture

Assigned: Unassigned » cecrs
cecrs’s picture

Assigned: cecrs » Unassigned
Niklas Fiekas’s picture

Assigned: Unassigned » Niklas Fiekas
Niklas Fiekas’s picture

Assigned: Niklas Fiekas » Unassigned
Status: Active » Needs review
FileSize
5.33 KB

A first stab at this.

vijaycs85’s picture

dawehner’s picture

Status: Needs review » Reviewed & tested by the community

It seems to be an INSANE idea to have a page to show the database query, so you can check it in your test, but yeah let's open a follow up to use a better approach.

+++ b/core/modules/node/tests/modules/node_access_test/node_access_test.moduleundefined
@@ -83,12 +83,6 @@ function node_access_test_permission() {
-  $items['node_access_test_page'] = array(
-    'title' => 'Node access test',
-    'page callback' => 'node_access_test_page',
-    'access arguments' => array('access content'),
-    'type' => MENU_SUGGESTED_ITEM,
-  );

Technically we don't remove the hook_menu entry if it is not a MENU_CALLBACK, but this is just about checking the page output.

alexpott’s picture

Title: Convert node_access_test_page() to a new style controller » Remove node_access_test_page()
Status: Reviewed & tested by the community » Needs work

It is totally insane to do the test this way.

Lets remove hook_menu, page callback and testNodeQueryAlterWithUI as this test is repeated with the low level tests.

juampynr’s picture

Status: Needs work » Needs review
FileSize
3.67 KB

Here you are.

dawehner’s picture

There is not only node_access_test_page but also node_access_entity_test_page so maybe we should get rid of that one as well.

juampynr’s picture

Sure! Closed #1987756: Remove node_access_entity_test_page().

Here is a patch which gets rid of both.

ParisLiakos’s picture

Status: Needs review » Reviewed & tested by the community

good riddance

alexpott’s picture

Status: Reviewed & tested by the community » Fixed

Committed 30a45ad and pushed to 8.x. Thanks!

Status: Fixed » Closed (fixed)

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

Berdir’s picture

This removed the only test method in that test, which means that it now spits out FATAL messages in the CLI test runner and the UI breaks: #2056293: Remove empty test Drupal\node\Tests\NodeEntityFieldQueryAlterTest