Shell out package#30
Conversation
This shell out package should be threated as a brigde solutions that will help Knative move out of using to much shell scripts. It will enable us using new Golang orchestrated tests, and build processes, and have ability to shell out to scripts if needed.
knative-prow-robot
added
the
size/XL
cardil
force-pushed
the
feature/shell-out
branch
from
8d616e2 to
03650a2
Compare
cardil
force-pushed
the
feature/shell-out
branch
from
e3fde83 to
d2e43fe
Compare
|
You can see a usage of this code in knative/eventing#4519 - here exactly knative/eventing#4519 test/upgrade/installation/shell.go@L24-L38. Here is an upgrade test prow job, in which those shell out can be seen: https://prow.knative.dev/view/gs/knative-prow/pr-logs/pull/knative_eventing/4519/pull-knative-eventing-upgrade-tests/1331636798266281984 |
* adding rigging, stage for merging * drop testbed, it was never finished. * adding a more complex test to show rigging for porting * adding e2e test * work with kind * rigging v2 impl, PoC * a working demo. * run the v2 example of rigging * clean up poc for v2 of rigging * revert pkg/test * cleanup * less debug * rework how flags are used * minor cleanup * show that you can also run more than one feature. * drop example code * adding options, minor cleanup * ville feedback * no need to nil check append on arrays
| package shell_test | ||
|
|
||
| import ( | ||
| "strings" | ||
| "testing" | ||
| ) | ||
|
|
||
| type assertions struct { | ||
| t *testing.T | ||
| } | ||
|
|
||
| func (a assertions) NoError(err error) { | ||
| if err != nil { | ||
| a.t.Error(err) | ||
| } | ||
| } | ||
|
|
||
| func (a assertions) Contains(haystack, needle string) { | ||
| if !strings.Contains(haystack, needle) { | ||
| a.t.Errorf("wanted to \ncontain: %#v\n in: %#v", | ||
| needle, haystack) | ||
| } | ||
| } | ||
|
|
||
| func (a assertions) Equal(want, got string) { | ||
| if got != want { | ||
| a.t.Errorf("want: %#v\n got:%#v", want, got) | ||
| } | ||
| } |
There was a problem hiding this comment.
Use testify for that?
There was a problem hiding this comment.
I wanted to avoid adding dependencies, as that opens a pandora box, of whether to vendor them or not.
There was a problem hiding this comment.
I guess this pandora box has to be opened at some point right?
There was a problem hiding this comment.
Not necessarily. In the ideal world I would see ALL contents of this repo, gradually being deprecated, and replaced by dedicated tools, and Golang code.
There was a problem hiding this comment.
exactly, to develop this golang code (eg embed other tools) you'll probably need managing dependencies anyway right?
There was a problem hiding this comment.
Yeah, But maybe in other repos then knative/hack?!? IDK
n3wscott
left a comment
n3wscott
left a comment
There was a problem hiding this comment.
/lgtm
/approve
/hold
Can you as a follow up or another commit to mark every exported method as // Deprecated: do not use this to avoid profiliation? I am hoping downstreams seeing the deprecated method in use will encourage other solutions.
Unhold if you want to do it as a followup. Thanks!!
knative-prow-robot
added
the
do-not-merge/hold
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: cardil, n3wscott The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
knative-prow-robot
added
the
approved
|
Thanks, @n3wscott! I'll do that as a follow up. I have an idea, of how to do it even more strict. /unhold |
knative-prow-robot
removed
the
do-not-merge/hold
* Migrate upgrade tests to use new framework * Lint fixes * Fixing shell outs * Removal of unneeded vandor * Lint fixes * Unit test fixes * Update deps to include knative/pkg#1912 * Fix unit test when running without proper git checkout * Using new shell-out package from knative.dev/hack * Fixing code style * Adjusting location of project file We need to point ourselves to specific directory as our scripts are location sensitive. * Bash scripts work with set -Eeuo pipefail * Upgrade knative.dev/hack to include knative/hack#30
4 participants
Fixes #28
This shell out package should be treated as a bridge solution that will help Knative move out of using too much shell scripts. It will enable us using new Golang orchestrated tests, and build processes, and have the ability to shell out to scripts if needed.