test/linux_target_test.go: refactor container tests into multiple suites #918
Conversation
invidian
force-pushed
the
invidian/linux-test-refactor
branch
3 times, most recently
from
25d637c to
05f6008
Compare
There was a problem hiding this comment.
Pull request overview
This PR refactors the large monolithic container test in test/linux_target_test.go into multiple focused test suites organized by functionality (build_steps, sources, artifacts, tests, container). This improves test organization, reduces complexity, and makes it easier to add new test cases. The PR also modifies JSON serialization tags in the Spec struct to support a spec merging pattern using JSON marshal/unmarshal.
Key Changes
- Test suite reorganization into logical groups: build_steps, sources, artifacts, tests, and container
- Introduction of
testLinuxSpechelper function that merges user-provided specs with a base spec using JSON serialization - Addition of
omitemptyto JSON tags for several Spec fields to enable partial spec serialization
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| test/linux_target_test.go | Refactors large container test into multiple focused test suites and adds testLinuxSpec helper for spec merging |
| spec.go | Adds omitempty to JSON tags for Name, Description, Website, Version, Revision, and License fields |
| docs/spec.schema.json | Updates schema to remove "website" and "license" from required fields and allows them to be null |
| Name string `yaml:"name" json:"name,omitempty" jsonschema:"required"` | ||
| // Description is a short description of the package. | ||
| Description string `yaml:"description" json:"description" jsonschema:"required"` | ||
| Description string `yaml:"description" json:"description,omitempty" jsonschema:"required"` | ||
| // Website is the URL to store in the metadata of the package. | ||
| Website string `yaml:"website" json:"website"` | ||
| Website string `yaml:"website" json:"website,omitempty"` | ||
|
|
||
| // Version sets the version of the package. | ||
| Version string `yaml:"version" json:"version" jsonschema:"required"` | ||
| Version string `yaml:"version" json:"version,omitempty" jsonschema:"required"` | ||
| // Revision sets the package revision. | ||
| // This will generally get merged into the package version when generating the package. | ||
| Revision string `yaml:"revision" json:"revision" jsonschema:"required,oneof_type=string;integer"` | ||
| Revision string `yaml:"revision" json:"revision,omitempty" jsonschema:"required,oneof_type=string;integer"` |
There was a problem hiding this comment.
Adding "omitempty" to fields marked with "jsonschema:"required"" (Name, Description, Version, Revision) creates a potential inconsistency. While this enables the spec merging pattern used in testLinuxSpec, it means that a marshaled Spec with empty required fields will omit those fields from JSON, potentially failing schema validation when these fields are truly required. Consider documenting this design decision or ensuring that validation occurs before/after merging operations to catch missing required fields.
invidian
force-pushed
the
invidian/linux-test-refactor
branch
from
05f6008 to
0fa8191
Compare
Signed-off-by: Mateusz Gozdek <mgozdek@microsoft.com>
This way we get more targeted tests and assertions, test scenarios complexity goes down and it makes it easier to extend for additional tests. Signed-off-by: Mateusz Gozdek <mgozdek@microsoft.com>
invidian
force-pushed
the
invidian/linux-test-refactor
branch
from
0fa8191 to
2935774
Compare
| }, | ||
| { | ||
| Command: "grep 'Added another new file' ./src2/file3", | ||
| Name: "Post-install symlinks should be created and have correct ownership", |
There was a problem hiding this comment.
Those tests are probably unnecessary here since they don't test source patches.
1 participant
This way we get more targeted tests and assertions, test scenarios
complexity goes down and it makes it easier to extend for additional
tests.
It also modifies some fields in Spec tags to omit them while serializing when they are empty, so we can use serialization for merging multiple specs, which I think is handy and I use it in tests to build up the spec from the basic one.