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

Testing framework for install-cni template processing #263

Merged

Conversation

marqc
Copy link
Member

@marqc marqc commented Jan 9, 2024

Adds tests to verify cni_spec generation by install-cni.sh script

@aojea
Copy link
Collaborator

aojea commented Jan 9, 2024

This is impressive, let´s get a run on CI

@aojea aojea marked this pull request as ready for review January 9, 2024 20:56
@aojea aojea requested a review from jingyuanliang January 9, 2024 20:57
@aojea
Copy link
Collaborator

aojea commented Jan 9, 2024

@jingyuanliang why are the githb actions not running?

@jingyuanliang
Copy link
Member

Add those new shell scripts to the target of shellcheck?

@aojea
Copy link
Collaborator

aojea commented Jan 9, 2024

Add those new shell scripts to the target of shellcheck?

those are tests, should we shelcheck the tests?

you are the experts here, it looks good to me but I´m very lazy with bash so you should review @jingyuanliang

@jingyuanliang
Copy link
Member

those are tests, should we shelcheck the tests?

Hmm I just had some nitpicks and wanted shellcheck to say it for me.

@jingyuanliang
Copy link
Member

Okay I just tried it. shellcheck gives many useless findings. Don't bother to add it then.

@marqc marqc force-pushed the install-cni-testing branch 2 times, most recently from b111771 to b2133ab Compare January 10, 2024 07:32
@marqc
Copy link
Member Author

marqc commented Jan 10, 2024

I decided to add it to shellcheck. Even if findings are not important or false-positive, having the check in pipeline can prevent us from introducing bugs later on.

I also want to add a readme file with documentation on the testing framework (how it works, how to use it). I will update the PR soon.

@marqc marqc force-pushed the install-cni-testing branch from b2133ab to 8f53f2e Compare January 10, 2024 14:08
@google-oss-prow google-oss-prow bot added size/XL and removed size/L labels Jan 10, 2024
@marqc
Copy link
Member Author

marqc commented Jan 12, 2024

@jingyuanliang please take a look, it's ready for review now

@jingyuanliang
Copy link
Member

I can think of several other test cases, but we can start with the current ones.

@marqc marqc force-pushed the install-cni-testing branch 4 times, most recently from fe75234 to 763be1f Compare January 17, 2024 11:10
Signed-off-by: Marek Chodor <[email protected]>
Change-Id: Idcacbffec8b385e3f22ca8262f4450f0b9b27e52
@marqc marqc force-pushed the install-cni-testing branch from 763be1f to 74ad02e Compare January 17, 2024 11:14
@marqc marqc requested a review from jingyuanliang January 17, 2024 11:15
@jingyuanliang
Copy link
Member

/lgtm
/approve

Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: jingyuanliang, marqc

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@google-oss-prow google-oss-prow bot merged commit 0423153 into GoogleCloudPlatform:master Jan 19, 2024
10 checks passed
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.

3 participants