-
Notifications
You must be signed in to change notification settings - Fork 471
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
V4 new #814
V4 new #814
Changes from all commits
85ce2c8
6c123f8
bb9c5a6
3f8f586
142c562
e787e01
28634ee
1929d66
bb7cb47
427348a
a6d2f2a
6bf1c78
4be04f9
dfb1186
12cef65
1fe5571
2ebb1f8
bf22f82
61922e6
14c3d45
db012e4
04eea68
2a8c9e5
ef62d85
aa990cf
5baad14
a83de2b
07f40df
46f455a
e2e0037
4639175
5815f53
28c1e53
b50a296
9e7b3db
ef7c933
d8c9c93
2a48c91
e7e7841
a77d2bb
a41dac2
d178d97
783a2b4
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -3,7 +3,7 @@ set -euo pipefail | |
|
||
command -v go | ||
|
||
go test -coverprofile="c.out" "${1}" | ||
go test -coverpkg=./... ./... -skip '^TestPrometheus' -coverprofile="c.out" | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Mind commenting the reason for the skip? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ooh, this one is easy.. it fails 😄 To be fair, I did run it on the main branch and it fails there too. Basically, the way that shell script was written it wasn't actually running those tests.. so they may have never worked.. not sure.. didn't try to check out the original commit to see. I can take a look as a follow-up PR, or since you're familiar with the code, maybe give it a second look? Maybe I broke something a while back and not noticed. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Hahaha, ok. Reasonably sure it ran and passed when introduced, but I also haven't gone back and checked. Once this is merged, I propose to spend some time enhancing our build & test pipeline. |
||
|
||
sed 's|github.com/goss-org/goss/||' <"c.out" >"c.out.tmp" | ||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -67,7 +67,6 @@ func timeoutFlag(value time.Duration) cli.DurationFlag { | |
} | ||
|
||
func main() { | ||
startTime := time.Now() | ||
app := cli.NewApp() | ||
app.EnableBashCompletion = true | ||
app.Version = version | ||
|
@@ -149,7 +148,7 @@ func main() { | |
}, | ||
Action: func(c *cli.Context) error { | ||
fatalAlphaIfNeeded(c) | ||
code, err := goss.Validate(newRuntimeConfigFromCLI(c), startTime) | ||
code, err := goss.Validate(newRuntimeConfigFromCLI(c)) | ||
if err != nil { | ||
color.Red(fmt.Sprintf("Error: %v\n", err)) | ||
} | ||
|
@@ -416,14 +415,14 @@ func addAlphaFlagIfNeeded(app *cli.App) { | |
if runtime.GOOS == "darwin" || runtime.GOOS == "windows" { | ||
app.Flags = append(app.Flags, cli.StringFlag{ | ||
Name: "use-alpha", | ||
Usage: fmt.Sprintf("goss is alpha-quality. Set to 1 to use anyway."), | ||
Usage: fmt.Sprintf("goss on macOS/Windows is alpha-quality. Set to 1 to use anyway."), | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Contradicts expectation from binary name? https://github.com/goss-org/goss/pull/814/files#diff-6ac3f79fc25d95cd1e3d51da53a4b21b939437392578a35ae8cd6d5366ca5485R44-R45 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I guess at this point I can remove the alpha flag altogether and just document that it's not as featureful/stable as Linux.. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Looking back at the history of this, I'll put it back on your plate.. What are your thoughts on alpha vs non-alpha. Your PR renamed the files.. seems they're fairly stable from your comment 2-3 years back. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'd be fine with renaming each binary to remove the |
||
EnvVar: "GOSS_USE_ALPHA", | ||
Value: "0", | ||
}) | ||
} | ||
} | ||
|
||
const msgFormat string = `WARNING: goss for this platform (%q) is alpha-quality, work-in-progress, and not yet exercised within continuous integration. | ||
const msgFormat string = `WARNING: goss for this platform (%q) is alpha-quality, work-in-progress and community-supported. | ||
|
||
You should not expect everything to work. Treat linux as the canonical behaviour to expect. | ||
|
||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same for windows...?