Problem/Motivation

Parent issue works as designed, we need to move along and try and execute: /Users/bjorn/projects/drupal/core/modules/jsonapi/tests/src/Functional/NodeTest.php:341. Which is a todo to remove some code when the issue is closed.

Steps to reproduce

Proposed resolution

Remove the code, enable new code as per comment in referenced line. See if all is good.

Remaining tasks

Remove the code
See all is happy and fine
...
Profit ;)

User interface changes

API changes

Data model changes

Release notes snippet

Issue fork drupal-3449891

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

About issue forks

Comments

bbrala created an issue. See original summary.

nexusnovaz’s picture

nexusnovaz commented
Assigned: Unassigned » nexusnovaz

Ill pick this one up

NexusNovaz opened merge request !8189

nexusnovaz’s picture

nexusnovaz commented
Assigned: nexusnovaz » Unassigned
Status: Active » Needs work

Hi, i think the Remaining tasks needs to be revised. I've removed the old test and uncommented the new one. The test can be viewed here with a failure.

bbrala’s picture

bbrala
Location Netherlands
commented

If you look at the arguments for assertResourceErrorResponse you see that your arguments to not correcpond with the right ones. The $response variable is in the wrong position. Check out the function arguments, and you should be able to fix that pretty easily by adding a NULL as the $via_link argument, or if that doesnt work you could also add the earlier $url there, that is probably better. :)

nexusnovaz’s picture

nexusnovaz commented

Hi @bbrala,

I'm unable to get the test to pass. I'm getting the following fail message with $url:

PHPUnit 9.6.19 by Sebastian Bergmann and contributors.

Runtime:       PHP 8.1.27
Configuration: ./core/phpunit.xml.dist

Testing Drupal\Tests\jsonapi\Functional\NodeTest
.F..........                                                      12 / 12 (100%)

Time: 01:16.183, Memory: 6.00 MB

There was 1 failure:

1) Drupal\Tests\jsonapi\Functional\NodeTest::testGetIndividual
The 'errors' member was not as expected.
Failed asserting that two arrays are equal.
--- Expected
+++ Actual
@@ @@
         1 => 'Forbidden'
         2 => 'The current user is not allow...ource.'
         3 => Array (
-            0 => Array (...)
-            1 => Array (...)
+            0 => '/data'
         )
+        4 => Array (...)
     )
 )

Or the following error message with NULL:

PHPUnit 9.6.19 by Sebastian Bergmann and contributors.

Runtime:       PHP 8.1.27
Configuration: ./core/phpunit.xml.dist

Testing Drupal\Tests\jsonapi\Functional\NodeTest
.F..........                                                      12 / 12 (100%)

Time: 01:17.307, Memory: 6.00 MB

There was 1 failure:

1) Drupal\Tests\jsonapi\Functional\NodeTest::testGetIndividual
The 'errors' member was not as expected.
Failed asserting that two arrays are equal.
--- Expected
+++ Actual
@@ @@
         1 => 'Forbidden'
         2 => 'The current user is not allow...ource.'
         3 => Array (
-            0 => Array (...)
+            0 => '/data'
         )
+        4 => Array (...)
     )
 )

As im unsure what to do, ill leave this for someone else and check up once complete to learn from the solution.

manish-31 made their first commit to this issue’s fork.

manish-31’s picture

manish-31 commented
Status: Needs work » Needs review

MR has been updated, fixed PHPUnit tests errors. Needs review.

smustgrave’s picture

smustgrave commented
Status: Needs review » Reviewed & tested by the community

Seems straightforward

Just fyi @manish-31 this was tagged for novice/new users. Looking at your post history of multiple pages believe are good to work on non-novice level issues.

longwave’s picture

longwave
he/him
Primary language English
Location UK
commented
Version: 11.x-dev » 10.3.x-dev
Status: Reviewed & tested by the community » Fixed

Committed and pushed 11ce3c837b to 11.x and 1dd4e98288 to 11.0.x and f5ecdb8f72 to 10.4.x and 3fd288da5c to 10.3.x. Thanks!

  • longwave committed 3fd288da on 10.3.x 
    Issue #3449891 by NexusNovaz, manish-31, bbrala: Move to new test path...

  • longwave committed f5ecdb8f on 10.4.x 
    Issue #3449891 by NexusNovaz, manish-31, bbrala: Move to new test path...

  • longwave committed 1dd4e982 on 11.0.x 
    Issue #3449891 by NexusNovaz, manish-31, bbrala: Move to new test path...

  • longwave committed 11ce3c83 on 11.x 
    Issue #3449891 by NexusNovaz, manish-31, bbrala: Move to new test path...

longwave closed merge request !8189

longwave’s picture

longwave
he/him
Primary language English
Location UK
commented

Status: Fixed » Closed (fixed)

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