-
Notifications
You must be signed in to change notification settings - Fork 472
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
Rename binaries to strip '-alpha' #671
Conversation
As discussed in goss-org#663 (comment); strip the -alpha naming, keep the messaging, keep the --use-alpha flag and env-var. This allows people to remove edge-cases from their installation, yet retains the clearly-set expectations of support being community-driven. Fixes goss-org#651.
e24db1c
to
726f047
Compare
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. |
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.
I figured that the integration tests running in CI merited this rewording.
@@ -408,14 +408,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 comment
The reason will be displayed to describe this comment to others. Learn more.
Even though this message should never display on other platforms, I figured this more clearly set expectations.
@@ -36,15 +36,13 @@ bench: | |||
$(info INFO: Starting build $@) | |||
go test -bench=. | |||
|
|||
alpha-test-%: release/goss-% | |||
test-int-validate-%: release/goss-% |
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.
I could equally have called this test-int-all-commands-%
and similarly run-command-tests.sh
but I thought that might have been confusing with the command
assertion specifically; what do you think, @aelsabbahy?
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.
no opinion on this currently.
I have some plans to re-write all of the integration tests.. but want to wait until after v4
@aelsabbahy I think this is ready to review; I'm just waiting for linux CI to pass. I expect it to, but then, I always expect it to... ;-) |
Awesome timing on this.. I'm really really hoping to have v4 released soon. Doing last-minute merges of breaking features and of-course there's going to be a massive rebase of the transform branch.. hopefully, it's good to go after that! |
Quick eyeball check on this looks good.. plus CI passed. Any idea why codeclimate didn't trigger? I assume none of your changes messed with that. I'll wait for your answer on codeclimate and merge.. If you didn't change anything, don't worry about why it's not working.. just let me know and I'll merge anyways. =) |
The only thing that springs to mind about codeclimate is maybe it's only configured to trigger against branches aimed at master, or only against branches aimed at protected branches? |
Yup, that's my guess too.. just wanted to check you didn't knowingly change anything to impact this. Related note, I may get rid of codeclimate, or at least run it against the official Go codebase and see how many findings come up. If Official Go codebase fails, then I'm not interested in codeclimate. Also, need to actually have golint fail the build and fix all those findings. Thanks for the PR, love the cleanups! |
* Migrate from go-ps to gopsutil for better process detection (#597) * Migrate from go-ps to gopsutil for better process detection * Remove debugging code * Mount (#601) * Add stale.yml config * Change stale.yml label * Add VfsOpts to mount * Use mountinfo filter * Fix build failures * Normalize seliux mount options for integration testing * Attempt to normalize tests across docker installs and fix osx test setup * Try different osx image * Rename binaries to strip '-alpha' (#671) * Rename binaries to strip '-alpha' As discussed in #663 (comment); strip the -alpha naming, keep the messaging, keep the --use-alpha flag and env-var. This allows people to remove edge-cases from their installation, yet retains the clearly-set expectations of support being community-driven. Fixes #651. * set expectations for future-us re: macOS+Windows vs linux * fix doc * F openrc runlevels (#668) * service: Add support for OpenRC runlevels * Omit empty runlevel * Update documentation * Update integration tests * Use consistent pattern for configu default values - Remove struct tag `default` value reflection * Make runLevels a testable attribute * empty commit to try to trigger travis * Update trusty to reflect precise changes Co-authored-by: Berney <[email protected]> * POC/DRAFT: Add transforms (#576) * Add transforms * wip add transforms work * Add some examples to make sense of this * Handle floats/ints better * Convert validatecontails to gomega * wip * Add gjson and contain-substring * wip * Subclass all matchers adding String() * I don't evne know what this is.. should have checked it in back when I wrote it * Migrate GomegaMatcher -> GossMatcher * lots of changes + drop json-iterator * Move output related logic to outputs * Add include_raw flag * Add bench output format * Update have_patterns to check input. Add stubs to make Not matcher act as omegaMatcher * SetEscapeHTML(false) for semver constraint * Ensure have-Key_with_value works * remove have-key-with-value * Fix error compact output. Add oMegaMatcher stubs. Ensure matcher vs m consistent * Cleanup matchers * Cleanup TestResult struct and remove need for dyno * Update all deps * Update tranformer tests and add summary line to bench output format * Update readme * Conditionally print missing and empty values. Sanitize found and expected values for consistency * Check value is valid before checking IsNil() * Initial docs change * WIP fix for pretty print * Swap order on video vs blog for dgoss docs * Updated all tests except for semver * Fix all unit tests * Fix detection of when matcher is set * Update documentation * Update docs/manual.md Co-authored-by: Peter Mounce <[email protected]> * Update docs/manual.md Co-authored-by: Peter Mounce <[email protected]> * gjson: Validate json before doing gets * Make errors more clear * Remove unused field * Bump go version * Update doc * Fix regression with gomega errors * Fix #262 no need to add file size by default * Fix output formats * Use proper quoting for windows test * Add error checking for gjson path not found Co-authored-by: Peter Mounce <[email protected]> * Fix mountinfo splitting when there's a quoted comma * Improve have_patterns error message * Add syntax checks for type casting * Rename file.contains to file.contents * ToString converter add suppert for []interface{} * Convert headers to lowercase. fixes #760 * Fix contains -> contents, lowercase headers and contain-element float64 * Enhance all resources to support key override closes #518, closes #742 * Sort output in documentation format closes #416 * Fix typo in contain_element message * Track start and end times per-test This also changes the way total time is calculated in the output summary. total time = endTime of last test - start time of first test * Cache test results in serve instead of output closes #612 * Use exit code 78 if test file is unparseable closes #317 * Full EVR for rpm and fix failing tests * more tests * Increase test coverage * Fixed numeric eq bug * Make output more consistent * Update dependencies * Fix some merge conflicts * Revert "Migrate from go-ps to gopsutil for better process detection (#597)" This reverts commit 85ce2c8. * Revert 1fe5571 change http.headers back to io.Reader * FIX: use filepath.Join() in order to be OS agnostic * FIX: only run matcher_tests on linux * FIX: replace failing www.microsoft.com test * FIX: trusty dockerfile merge regression * Run all tests (except failing prometheus) as part of CI. Seems they were not running * Remove bench output, add some comments * Changed: include_raw by default, provide exclude_raw as a format flag * Changed: go-funk -> lo * Changed: temp fix to deal with httpbin.org slowness. Need to move to offline testing to avoid flakiness * Locking down version in install.sh, to avoid RC being installed --------- Co-authored-by: Peter Mounce <[email protected]> Co-authored-by: Berney <[email protected]> Co-authored-by: Peter Mounce <[email protected]>
Checklist
make test-all
(UNIX) passes. CI will also test thisDescription of change
As discussed in #663 (comment); strip the -alpha naming, keep the messaging, keep the --use-alpha flag and env-var.
We're reasonably comfortable with the current situation - nothing seems to be horrendously broken in terms of the ported functionality as exercised by the added macOS & Windows integration tests, and based on Improbable's use (that I am intimately familiar with).
This change allows people to remove edge-cases from their installation, yet retains the clearly-set expectations of support being community-driven.
Fixes #651. Continues #663.
Note: this is aimed at
v4
.