Problem/Motivation

Currently, the EntitySerializationTest test does not register all Normalizers with its mock Serializer. This means we aren't testing it as it would work in a full install.

Proposed resolution

Switch to DrupalUnitTestBase which creates a mock container. Use the serializer from this to test.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Anonymous’s picture

Status: Active » Needs review
FileSize
5.89 KB
25.13 KB
Anonymous’s picture

Category: task » bug
Issue tags: +serialization
damiankloip’s picture

Status: Needs review » Needs work

I like the test helper! This makes the tests much easier to read.

+++ b/core/modules/serialization/lib/Drupal/serialization/Tests/SerializationTestSetupHelper.phpundefined
@@ -0,0 +1,101 @@
+  protected function encoders() {
...
+  protected function normalizers() {

This looks confusing in the constructor, maybe we can just change the names of these to initEncoders/initNormalizers?

Anonymous’s picture

In working with DrupalUnitTestBase to improve JSON-LD tests, I realized that it actually creates a mock container anyways. We can use the mock container that it provides instead of creating a helper class here. Will rewrite the patch shortly.

damiankloip’s picture

Lin, are you picking this one up? If not, I am happy to do this today.

Anonymous’s picture

Assigned: linclark » Unassigned

It took me longer than I expected to debug the DrupalUnitTestBase issue in the JSON-LD tests, so I didn't get a chance to do this yesterday. Feel free to tackle it :)

damiankloip’s picture

Status: Needs work » Needs review
FileSize
5.1 KB

We can actually convert these tests to use DrupalUnitTestBase too, so let's try that. It worked fine, apart from the default_langcode, which I have changed the expected data for.

I actually checked this normally, outside of tests, and it matches the structure that I have changed the expected data to in the test. SO I'm inclined to think this might be right. What do you think?

Cuts the text runs from ~10 secs to ~3 secs, not bad!

damiankloip’s picture

FileSize
5.11 KB

Just another reroll now that we are in the serialization module.

Anonymous’s picture

Since entity_test is enabled manually, it doesn't need to be in the modules array.

Other than that, this looks good.

damiankloip’s picture

FileSize
659 bytes
5.28 KB

Good point, thanks for the review.

Anonymous’s picture

Status: Needs review » Reviewed & tested by the community

Looks good to me!

Anonymous’s picture

Component: base system » serialization.module
catch’s picture

Status: Reviewed & tested by the community » Fixed

Committed/pushed to 8.x, thanks!

tim.plunkett’s picture

Title: Update EntitySerializationTest to reflect actual Serializer setup » HEAD BROKEN: Update EntitySerializationTest to reflect actual Serializer setup
Priority: Normal » Critical
Status: Fixed » Active
tim.plunkett’s picture

Status: Active » Needs review
FileSize
1.73 KB

Passes locally.

catch’s picture

Status: Needs review » Fixed

Committed/pushed the follow-up, whoopsie that one was definitely me since I committed the DUTB patch yesterday...

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

Anonymous’s picture

Issue summary: View changes

Updated with new proposed solution.