Skip to content
This repository has been archived by the owner on Feb 24, 2020. It is now read-only.

tests: use appc schema instead of string templates #3520

Merged
merged 1 commit into from
Jan 13, 2017
Merged

tests: use appc schema instead of string templates #3520

merged 1 commit into from
Jan 13, 2017

Conversation

ybubnov
Copy link
Contributor

@ybubnov ybubnov commented Jan 7, 2017

This patch introduces a new package "pkg/aci/acitest" used to set the
default parameters to the application container image manifest
(precisely: name, version, kind).

It replaces the strings templates used to mock the manifest's JSON to
the types from "github.com/appc/schema" go-package.

Fixes #714

@ghost
Copy link

ghost commented Jan 7, 2017

Can one of the admins verify this patch?

@s-urbaniak
Copy link
Contributor

ok to test

@s-urbaniak s-urbaniak added this to the v1.23.0 milestone Jan 9, 2017
return NewACI(dir, manifest, nil)
manifest := schema.ImageManifest{
ACKind: schema.ImageManifestKind,
ACVersion: SpecVersion,
Copy link
Member

@lucab lucab Jan 10, 2017

Choose a reason for hiding this comment

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

Appc version labels use to be updated via a script. Can we turn this back into a string and use NewSemVer()?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

You mean, the SpecVersion variable, like this:

var SpecVersion, _ = types.NewSemVer("0.8.9")

Copy link
Member

Choose a reason for hiding this comment

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

Yes, sorry if I was a bit cryptic.

@lucab
Copy link
Member

lucab commented Jan 10, 2017

@ybubnov thank you very much for this PR. Just a minor request to make the version label manageable by script, otherwise looks fine 👍.

@lucab
Copy link
Member

lucab commented Jan 10, 2017

@jonboulle is there anything else missing from #714 after this?

@jonboulle
Copy link
Contributor

Seems good to me!

@ybubnov
Copy link
Contributor Author

ybubnov commented Jan 11, 2017

Addressed the review comments and squashed the changes into the single commit.

@lucab
Copy link
Member

lucab commented Jan 11, 2017

Ah, I forgot about AppContainerVersion, even better.

This patch introduces a new package "pkg/aci/acitest" used to set the
default parameters to the application container image manifest
(precisely: name, version, kind).

It replaces the strings templates used to mock the manifest's JSON to
the types from "github.com/appc/schema" go-package.
Copy link
Member

@lucab lucab left a comment

Choose a reason for hiding this comment

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

LGTM and thanks a lot for the cleanup!

@lucab lucab merged commit 1ecd718 into rkt:master Jan 13, 2017
@ybubnov ybubnov deleted the acitest-helpers-instead-of-strings-in-tests branch January 13, 2017 15:51
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants