Skip to content

bats: Fix spec validation test#900

Merged
cyphar merged 1 commit into
opencontainers:masterfrom
mrunalp:fix_test
Jun 10, 2016
Merged

bats: Fix spec validation test#900
cyphar merged 1 commit into
opencontainers:masterfrom
mrunalp:fix_test

Conversation

@mrunalp

@mrunalp mrunalp commented Jun 9, 2016

Copy link
Copy Markdown
Contributor

Use the commit that we have in Godeps.json rather than relying on
runtime-spec HEAD

Signed-off-by: Mrunal Patel mrunalp@gmail.com

@mrunalp

mrunalp commented Jun 9, 2016

Copy link
Copy Markdown
Contributor Author

https://github.com/opencontainers/runtime-spec/pull/481/files broke spec validation. This PR should make the test more reliable.

@mrunalp

mrunalp commented Jun 9, 2016

Copy link
Copy Markdown
Contributor Author

@crosbymichael @LK4D4 PTAL. Tests are broken right now without fixing this.

Comment thread tests/integration/spec.bats Outdated
run git reset --hard $SPEC_COMMIT
[ "$status" -eq 0 ]

cd "$HELLO_BUNDLE"

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Instead of cding in and back, you can use something like run git -C src/runtime-spec reset --hard "${SPEC_COMMIT}".

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

done

@mrunalp
mrunalp force-pushed the fix_test branch 2 times, most recently from fb2c7a9 to 9b6469d Compare June 9, 2016 19:19
[ "$status" -eq 0 ]

SPEC_COMMIT=$(grep runtime-spec ${TESTDIR}/../../Godeps/Godeps.json -A 4 | grep Rev | cut -d":" -f 2 | tr -d ' "')
run git -C src/runtime-spec reset --hard "${SPEC_COMMIT}"

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I'm not familiar with bats, but I think you want to check the exit status of the Git command with [ "$status -eq 0]. The existence of schema.json isn't sufficient to tell if the reset succeeded.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

done.

Use the commit that we have in Godeps.json rather than relying on
runtime-spec HEAD

Signed-off-by: Mrunal Patel <mrunalp@gmail.com>
@wking

wking commented Jun 9, 2016 via email

Copy link
Copy Markdown
Contributor

@crosbymichael

crosbymichael commented Jun 9, 2016

Copy link
Copy Markdown
Member

LGTM

Approved with PullApprove

run git clone https://github.com/opencontainers/runtime-spec.git src/runtime-spec
[ "$status" -eq 0 ]

SPEC_COMMIT=$(grep runtime-spec ${TESTDIR}/../../Godeps/Godeps.json -A 4 | grep Rev | cut -d":" -f 2 | tr -d ' "')

@cyphar cyphar Jun 10, 2016

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I'm guessing we can't get these files from Godep/* because we clean them? Whatever, we can fix this is in a followup PR. I don't like the brittle -A 4 though.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Yeah, those files aren't in Godeps. We can clean up -A 4 with either python or jq. I'll create a follow up. Wanted to fix the build fast.

@cyphar

cyphar commented Jun 10, 2016

Copy link
Copy Markdown
Member

LGTM, but we need to fix this in a more nice way (-A 4 seems quite brittle to me).

Approved with PullApprove

@cyphar
cyphar merged commit e40e71f into opencontainers:master Jun 10, 2016
stefanberger pushed a commit to stefanberger/runc that referenced this pull request Sep 8, 2017
…ation

*: Add "Initiative" to OCI Runtime Specification and expand OCI
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.

5 participants