- πΊπΈUnited States m.stenta
m.stenta β made their first commit to this issueβs fork.
- πΊπΈUnited States m.stenta
I've started working on adding basic test coverage in MR !15. These first set of tests are passing.
There is more I would like to do, so I'm setting this to "Needs Work", but I want to push what I have now and document some things.
As of right now, I've pushed 5 commits. Here is what each aims to achieve:
- Initial JSON:API Schema kernel test. - This just creates an empty kernel test with dependencies, to ensure that simply installing the module works.
- Test that /jsonapi/schema is available and has the expected content. - This tests the entrypoint (
/jsonapi/schema
) schema content. It checks to make sure all expected resource types are listed, and all expected meta data is present. - Test that resource collection schemas are generated. - This focuses on testing the
/jsonapi/[entity-type]/[bundle]/collection/schema
output. These are relatively simple and standardized across all entity types, as far as I'm aware, so the tests aren't complicated. - Test that resource schemas are generated. - This focused on testing the
/jsonapi/[entity-type]/[bundle]/resource/schema
output, but only with a focus on the standard/common elements across all resource types. - Add tests for a basic Article node type. - This starts to drill down into more specific details of a single resource type (an article node type without any custom fields). This is where we start to test some of the more specific aspects of attributes and relationships.
- Refactor $expected_resources so that it is not order dependent. - I found that the order of resources differed between my local development environment and Drupal.org GitLab, which caused my tests to fail. This commit simply refactors the tests so they aren't dependent on the order of resources.
The next things I want to test are:
- Installing the Field module - I started this but found that simply adding the Field module causes an unusual error (seems related to π Decimal field causes error message on custom entity Active ). This requires more digging to figure out what's happening.
- Test required relationships - Right now there is test to check that "required"
attributes
are listed (thetitle
attribute of the node is required by default), but there are no tests for requiredrelationships
. I will add anentity_reference
field to the article node type and make it required to test this. - Test other field types - The basic node type allows us to test a few things already (date, string, boolean fields, etc), but in order to test other field types we'll need to create more config fields. So I need to debug the
field
module installation (above) first.
Important context: Normally test driven development (TDD) would involve writing tests first to lay out the expectations, and then writing the code to meet those expectations. Since all of the current code was written without tests, I am approaching these tests from the other direction. I'm looking at the JSON Schema that the module currently outputs, and writing the expectation tests around that. It's important to understand what this means: if the current output is bad (eg: doesn't meet JSON Schema spec), then these tests are not going to catch those issues. They are primarily going to confirm that the module is working the way it does currently, good or bad. In an ideal world we would have approached this the other way around, but this feels like the best way forward from this point. Adding these tests will allow us to more easily fix other bugs that have been reported, and we can update/improve the tests accordingly as we go. They will also make it easier to confirm that the module works on Drupal 11 (see π Automated Drupal 11 compatibility fixes for jsonapi_schema Needs review ).
- πΊπΈUnited States m.stenta
It's important to understand what this means: if the current output is bad (eg: doesn't meet JSON Schema spec), then these tests are not going to catch those issues. They are primarily going to confirm that the module is working the way it does currently, good or bad.
Adding these tests has already revealed one inconsistency that probably deserves it's own issue. It seems that a lot of config entity types do not declare
data.definitions.attributes.type
(normally "object
") ordata.definitions.attributes.description
(normally "Entity attributes
") in their schema. For example, look at/jsonapi/entity_view_mode/entity_view_mode/resource/schema
, and you'll see thatdata.definitions.attributes
only hasproperties
andadditionalProperties
sub-elements. But if you look at/jsonapi/user_role/user_role/resource/schema
it also hastype
anddescription
.For now, I've added a condition to the tests to ignore the known problematic entity types, but ideally we can fix that and remove that condition in the future.
- πΊπΈUnited States m.stenta
Adding these tests has already revealed one inconsistency that probably deserves it's own issue.
Oh I wonder if this is related: #3324769: Schema incorrect for config entity types with multi-part IDs (field_storage_config, field_config, entity_view_mode, entity_form_mode, entity_view_display and entity_form_display) β
- πΊπΈUnited States m.stenta
Installing the Field module - I started this but found that simply adding the Field module causes an unusual error (seems related to π Decimal field causes error message on custom entity Active ). This requires more digging to figure out what's happening.
Ah this turned out to be an issue because I was running tests with the changes necessary for Drupal 11 ( π Automated Drupal 11 compatibility fixes for jsonapi_schema Needs review ). When I run against 8.x-1.x this works fine. This is something we will have to fix for D11, though, because that explicitly declares the return types of normalizer methods, and this module returns an invalid type (
stdClass
) in some cases. That's a separate issue... - πΊπΈUnited States m.stenta
Test required relationships
Test other field typesThese are done. Tests are green. I think we can consider this complete! Good enough for a start, anyway. :-)
- πΊπΈUnited States m.stenta
I forgot about the "related" schema endpoints (
). I pushed some commits that add tests for them, as well as some general test reorganization.
I'm going to go ahead and merge this so that I can start using it to test/fix/merge other bugs/patches in the issue queue.
-
m.stenta β
committed 56118a9d on 8.x-1.x
Issue #3257911: Add basic test coverage
-
m.stenta β
committed 56118a9d on 8.x-1.x