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

[#439] Add --vars-inline that is capable of overwriting --vars #534

Merged
merged 5 commits into from Feb 12, 2020

Conversation

@pedroMMM
Copy link
Contributor

pedroMMM commented Jan 2, 2020

Checklist
  • make test-all (UNIX) passes. CI will also test this
  • unit and/or integration tests are included (if applicable)
  • documentation is changed or added (if applicable)

Description of change

Add --vars-inline to dynamically pass variables which can overwrite variables sourced from --vars, but with no deep merging.

Closes #439

@aelsabbahy

This comment has been minimized.

Copy link
Owner

aelsabbahy commented Jan 2, 2020

I'll review this over the weekend. At first glance tests/results it looks great.

@aelsabbahy

This comment has been minimized.

Copy link
Owner

aelsabbahy commented Feb 4, 2020

So, trying this out, and seems like there's either a bug in the code, or I'm misunderstanding it.
goss.yaml

package:
  coreutils:
    installed: true
$ goss --vars-inline '{"package": "coreutils"}' render
panic: assignment to entry in nil map

goroutine 1 [running]:
github.com/aelsabbahy/goss.NewTemplateFilter(0x0, 0x0, 0x7ffc125b2ee8, 0x18, 0x18)
        /home/aelsabbahy/sandbox/golang/src/github.com/aelsabbahy/goss/template.go:68 +0x1cf
github.com/aelsabbahy/goss.RenderJSON(0xc0000d4dc0, 0xc0000b272f, 0x9de627)
        /home/aelsabbahy/sandbox/golang/src/github.com/aelsabbahy/goss/store.go:137 +0x162
main.main.func3(0xc0000d4dc0, 0xc0000b6800, 0xc0000d4d00)
        /home/aelsabbahy/sandbox/golang/src/github.com/aelsabbahy/goss/cmd/goss/goss.go:153 +0x2f
github.com/urfave/cli.HandleAction(0x92ba60, 0xa02330, 0xc0000d4dc0, 0xc0000b6800, 0x0)
        /home/aelsabbahy/sandbox/golang/pkg/mod/github.com/urfave/cli@v0.0.0-20161102131801-d86a009f5e13/app.go:471 +0xad
github.com/urfave/cli.Command.Run(0x9ded3b, 0x6, 0x0, 0x0, 0xc000093f50, 0x1, 0x1, 0x9ea921, 0x1d, 0x0, ...)
        /home/aelsabbahy/sandbox/golang/pkg/mod/github.com/urfave/cli@v0.0.0-20161102131801-d86a009f5e13/command.go:191 +0x924
github.com/urfave/cli.(*App).Run(0xc0000a1860, 0xc0000b8040, 0x4, 0x4, 0x0, 0x0)
        /home/aelsabbahy/sandbox/golang/pkg/mod/github.com/urfave/cli@v0.0.0-20161102131801-d86a009f5e13/app.go:241 +0x654
main.main()
        /home/aelsabbahy/sandbox/golang/src/github.com/aelsabbahy/goss/cmd/goss/goss.go:343 +0x1e68

I tried:

--vars-inline '{"package": "coreutils"}'
--vars-inline '{package: coreutils}'
--vars-inline 'package: coreutils'
@aelsabbahy

This comment has been minimized.

Copy link
Owner

aelsabbahy commented Feb 4, 2020

Oh, found the bug! seems --vars-inline doesn't work if --vars isn't provided

@pedroMMM

This comment has been minimized.

Copy link
Contributor Author

pedroMMM commented Feb 4, 2020

Great catch! Thank you, I will address this bug as soon I have some time. I have been very busy with repairs at the new house...

@aelsabbahy

This comment has been minimized.

Copy link
Owner

aelsabbahy commented Feb 4, 2020

Congrats on the new house. :)

pedroMMM added 2 commits Jan 2, 2020
@pedroMMM pedroMMM force-pushed the pedroMMM:issue/439 branch from e9e37b0 to b202ba6 Feb 11, 2020
@pedroMMM pedroMMM force-pushed the pedroMMM:issue/439 branch from 6b1b8e3 to c7e44bb Feb 11, 2020
@pedroMMM

This comment has been minimized.

Copy link
Contributor Author

pedroMMM commented Feb 11, 2020

Thank you @aelsabbahy for the patience, it's ready to be reviewed again. I finally had some uninterrupted free time to wrap my head around Goss.

I abstracted the calls to load the variables and merge them and unit test it to ensure no other problems from now on.

I got problems on Code Climate so I tweaked the TemplateFilter in the template.go to satisfy Code Climate and simplify efforts like the one raised recently on #544.

Copy link
Owner

aelsabbahy left a comment

Looks great, small change to env var naming and then it's ready to go.

Thanks for the great contributions!

cmd/goss/goss.go Outdated Show resolved Hide resolved
docs/manual.md Outdated Show resolved Hide resolved
… syntax

Co-Authored-By: Ahmed Elsabbahy <aelsabbahy@users.noreply.github.com>
@pedroMMM pedroMMM requested a review from aelsabbahy Feb 12, 2020
@pedroMMM pedroMMM mentioned this pull request Feb 12, 2020
1 of 1 task complete
@aelsabbahy aelsabbahy merged commit 6cb041d into aelsabbahy:master Feb 12, 2020
2 checks passed
2 checks passed
codeclimate 7 fixed issues
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

2 participants
You can’t perform that action at this time.