Skip to content

bats: Fix spec validation test#900

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

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

Conversation

@mrunalp
Copy link
Copy Markdown
Contributor

@mrunalp mrunalp commented Jun 9, 2016

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
Copy link
Copy Markdown
Contributor Author

mrunalp commented Jun 9, 2016

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

@mrunalp
Copy link
Copy Markdown
Contributor Author

mrunalp commented Jun 9, 2016

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

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
Copy link
Copy Markdown
Contributor

wking commented Jun 9, 2016 via email

@crosbymichael
Copy link
Copy Markdown
Member

crosbymichael commented Jun 9, 2016

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 ' "')
Copy link
Copy Markdown
Member

@cyphar cyphar Jun 10, 2016

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
Copy link
Copy Markdown
Member

cyphar commented Jun 10, 2016

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