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.

Sign up for GitHub

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

Jump to bottom

Script for generating unit tests #805

Merged
merged 1 commit into from
Dec 19, 2017
Merged

Script for generating unit tests #805

merged 1 commit into from
Dec 19, 2017

Conversation

ashetty1
Copy link
Contributor

@ashetty1 ashetty1 commented Sep 1, 2017 •
edited
Loading

Issue #770

@k8s-ci-robot k8s-ci-robot added cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Sep 1, 2017
@ashetty1
Copy link
Contributor Author

ashetty1 commented Sep 1, 2017 •
edited
Loading

@cdrage Please see if this works. Will proceed based on your inputs.

@cdrage
Copy link
Member

cdrage commented Sep 1, 2017

This works, but perhaps lets make it interactive? Run ./script/make-test.sh or something and it interacts with you and asks for the test directory?

cdrage
cdrage requested changes Sep 1, 2017
View reviewed changes
script/test/cmd/generate_unit.sh Outdated
TEST_DIR=''

# Location of the docker-compose file
COMPOSE_FILE="${TEST_DIR}/docker-compose.yml"
Copy link
Member

Choose a reason for hiding this comment

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

most files use yaml I believe not yml

script/test/cmd/generate_unit.sh Outdated
}

generate_os() {
kompose convert --provider=openshift -f $COMPOSE_FILE -j -o $TEST_DIR/output-os.json
Copy link
Member

Choose a reason for hiding this comment

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

please use ./kompose

script/test/cmd/generate_unit.sh Outdated
fi

generate_k8s() {
kompose convert -f $COMPOSE_FILE -j -o $TEST_DIR/output-k8s.json
Copy link
Member

Choose a reason for hiding this comment

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

please use ./kompose

@ashetty1
Copy link
Contributor Author

ashetty1 commented Sep 6, 2017

@cdrage @surajnarwade review please

@surajnarwade
Copy link
Contributor

@ashetty1 , can you add it in Makefile and bit of documentation, I am confused how to test it

@k8s-ci-robot k8s-ci-robot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Sep 11, 2017
@ashetty1
Copy link
Contributor Author

@surajnarwade Not sure if we should add it to Makefile. This is not a user-facing script.

@ashetty1 ashetty1 changed the title [WIP] Script for generating unit tests Script for generating unit tests Sep 18, 2017
@cdrage
Copy link
Member

cdrage commented Sep 26, 2017 •
edited
Loading

IMO, let's add it to makefile, it doesn't hurt adding more stuff to it. @ashetty1

@kadel
Copy link
Member

kadel commented Oct 2, 2017

@surajnarwade Not sure if we should add it to Makefile. This is not a user-facing script.

It is not user-facing, but it is Kompose developer facing. This needs to be documented in Development Guide

@kadel
Copy link
Member

kadel commented Oct 2, 2017

IMO, let's add it to makefile, it doesn't hurt adding more stuff to it. @ashetty1
👍 1

+1 for adding this to Makefile, otherwise it will be hard to find those scripts, it is much more convenient to run this from make

@k8s-ci-robot k8s-ci-robot added size/M Denotes a PR that changes 30-99 lines, ignoring generated files. and removed size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Oct 3, 2017
@cdrage
Copy link
Member

cdrage commented Oct 3, 2017 •
edited
Loading

@ashetty1 please merge commits into one 👍

cdrage
cdrage requested changes Oct 10, 2017
View reviewed changes
Makefile Outdated
@@ -66,6 +66,11 @@ test-openshift:
test-cmd:
./script/test/cmd/tests.sh

# generate commandline tests
.PHONY: gen-cmd
gen-cmd:
Copy link
Member

Choose a reason for hiding this comment

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

may be better to do generate-test-cmd ? wdyt @kadel

@ashetty1
Script for generating CLI tests:
7bb6767 
* Made an entry in the Makefile: `make gen-cmd` will run the script now

* Added a section on adding CLI tests in development docs
@cdrage cdrage mentioned this pull request Nov 8, 2017
Merged
@cdrage
Copy link
Member

cdrage commented Dec 19, 2017

This LGTM. Let's merge and go from there.

@cdrage cdrage merged commit e6a40eb into kubernetes:master Dec 19, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@cdrage cdrage cdrage approved these changes

Assignees
No one assigned
Labels
cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. size/M Denotes a PR that changes 30-99 lines, ignoring generated files.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
5 participants
@ashetty1 @cdrage @surajnarwade @kadel @k8s-ci-robot