elastic / package-spec

EPR package specifications
Other
17 stars 70 forks source link

Refactor tests for field definitions #700

Closed jsoriano closed 7 months ago

jsoriano commented 7 months ago

What does this PR do?

Adds a new test for field definitions, whose cases receive as inputs a template package, a set of fields definitions and a list of expected errors. The test builds for each case a new package from the template package and inserts the field definitions, then it runs the validations and checks for the expected errors. A test case looks like this now:

                {
                        title:           "required object type",
                        packageTemplate: "integration_v3_0",
                        fields: []map[string]any{
                                {
                                        "name": "object_without_object_type.*",
                                        "type": "object",
                                },
                        },
                        expectedErrors: []string{
                                `field 0: object_type is required`,
                        },
                },

Test packages whose only purpose was to test field definitions have been removed and their test cases migrated to the new test.

I have decided to validate complete packages so we also validate against semantic rules.

Why is it important?

This approach makes easier to add new test cases for fields. With previous implementation we need to add fields to a package and then add the list of expected errors to a test, with the new approach the field definitions to be tested and the expected errors are together in the same test case. Base packages can be more easily reused, so we don't need to add new packages for each new feature we want to test.

Checklist

Related issues

jsoriano commented 7 months ago

@mrodm added the option to override the spec version. It has been a bit rabbit hole because we have to modify also the changelog, and take into account that using non-GA versions of the spec also requires the package to have a non-GA version.

I also added an option to apply patches to files. It only supports appending, but is all we need by now.

elasticmachine commented 7 months ago

:green_heart: Build Succeeded

History

cc @jsoriano

mrodm commented 7 months ago

@mrodm added the option to override the spec version. It has been a bit rabbit hole because we have to modify also the changelog, and take into account that using non-GA versions of the spec also requires the package to have a non-GA version.

I also added an option to apply patches to files. It only supports appending, but is all we need by now.

Oh! I didn't forsee that it would require that many changes, thanks @jsoriano!!