Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fixes fixture test for build context #577

Merged
merged 1 commit into from
May 9, 2017

Conversation

surajnarwade
Copy link
Contributor

Resolves #576

This PR includes output-os-template.json in nginx-node-redis example,
which is basically output template contains %URI% and %REF% variables
which will be filled while initializing test cases and new will be stored as
/tmp/output-os.json
This will remove the problem of git uri and ref.

@k8s-ci-robot k8s-ci-robot added the cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. label Apr 27, 2017
@surajssd
Copy link
Member

surajssd commented May 2, 2017

Tests fail?

@surajnarwade surajnarwade force-pushed the fix-build-test branch 14 times, most recently from a14699e to cd64d2a Compare May 4, 2017 09:23
@surajnarwade
Copy link
Contributor Author

cc @cdrage @surajssd

# Only do something if the HEAD is detached.
if [ $(git rev-parse --abbrev-ref HEAD) == 'HEAD' ]; then
# If the current detached HEAD equals the master, then check out the master.
[[ $(git rev-parse HEAD) == $(git rev-parse master) ]] && git checkout master
Copy link
Member

Choose a reason for hiding this comment

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

not tested, but i think changing to master will not test the current PR

@@ -0,0 +1,14 @@
#!/usr/bin/env bash

Copy link
Member

Choose a reason for hiding this comment

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

Would be good to leave a little note where this is coming from :) Source.

Also, missing license!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@cdrage added license and note

# See the License for the specific language governing permissions and
# limitations under the License.

#Reference Link: https://github.com/theochem/horton/blob/master/tools/qa/fix_detached_head.sh
Copy link
Member

Choose a reason for hiding this comment

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

Missing space

@cdrage
Copy link
Member

cdrage commented May 8, 2017

Other than my comment, LGTM.

Side note: Can't wait for #89 is fixed so we don't run into these issues in the future 👍

@surajnarwade
Copy link
Contributor Author

@cdrage done

.travis.yml Outdated
@@ -17,6 +17,7 @@ matrix:

install:
- true
- script/test/cmd/fix_detached_head.sh
Copy link
Member

Choose a reason for hiding this comment

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

Argh. I should of added a comment here.

Can you put:

// This is required because sometimes Travis ends up in a detached HEAD state after git clone

?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yeah sure

Resolves kubernetes#576

This PR includes `output-os-template.json` in `nginx-node-redis` example,
which is basically output template contains `%URI%` and `%REF%` variables
which will be filled while initializing test cases and new will be stored as
`/tmp/output-os.json`
This will remove the problem of git uri and ref.
Copy link
Member

@surajssd surajssd left a comment

Choose a reason for hiding this comment

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

LGTM!

@surajssd surajssd merged commit ebd9dcf into kubernetes:master May 9, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cncf-cla: yes Indicates the PR's author has signed the CNCF CLA.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants