Skip to content

api/server/httputils: DecodePlatform: improve test-coverage#48680

Merged
thaJeztah merged 1 commit intomoby:masterfrom
thaJeztah:decodeplatform_coverage
Oct 17, 2024
Merged

api/server/httputils: DecodePlatform: improve test-coverage#48680
thaJeztah merged 1 commit intomoby:masterfrom
thaJeztah:decodeplatform_coverage

Conversation

@thaJeztah
Copy link
Copy Markdown
Member

  • Use fixtures for the JSON strings
  • Add test-cases for invalid / malformed JSON
  • Check error-message produced
  • Add test for "happy path"

- Use fixtures for the JSON strings
- Add test-cases for invalid / malformed JSON
- Check error-message produced
- Add test for "happy path"

Signed-off-by: Sebastiaan van Stijn <[email protected]>
Comment on lines +121 to +126
expectedErr: `failed to parse platform: unexpected end of JSON input`,
},
{
doc: "not JSON",
platformJSON: `linux/ams64`,
expectedErr: `failed to parse platform: invalid character 'l' looking for beginning of value`,
Copy link
Copy Markdown
Member Author

@thaJeztah thaJeztah Oct 17, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

FWIW; planning to make some improvements here after this;

  • Friendlier messages for invalid / non-JSON
  • Considering os to be required, but possibly (TBD) architecture to be optional,
  • ☝️ but: if a variant is specified, then architecture is required
  • Considering os.features to be rejected; we currently don't use it for platform-matching, so it may be better to reject it until we do

Basically make the (from a CLI perspective) accepted format to be;

<os>[(os.version)][/architecture[/variant]]

@thaJeztah thaJeztah requested a review from vvoland October 17, 2024 10:21
@thaJeztah thaJeztah merged commit 3900f9a into moby:master Oct 17, 2024
@thaJeztah thaJeztah deleted the decodeplatform_coverage branch October 17, 2024 12:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants