-
Notifications
You must be signed in to change notification settings - Fork 40
Fail 'jf scan' when a wrong flag is provided after command's arguments #165
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
Fail 'jf scan' when a wrong flag is provided after command's arguments #165
Conversation
…with arguments that have been provided in an incorrect order
attiasas
left a comment
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.
Nice, take a look at my comments
…o fix-scan-ignored-flags-issue
…o fix-scan-ignored-flags-issue
…o fix-scan-ignored-flags-issue
|
Please add integration tests you think might simulate it - with the common mistakes we have seen (flags at the end instead of beginning, |
…o fix-scan-ignored-flags-issue # Conflicts: # utils/utils.go
…o fix-scan-ignored-flags-issue
Merging this branch will not change overall coverage
Coverage by fileChanged files (no unit tests)
Please note that the "Total", "Covered", and "Missed" counts above refer to code statements instead of lines of code. The value in brackets refers to the test coverage of that file in the old version of the code. Changed unit test files
|
…o fix-scan-ignored-flags-issue # Conflicts: # scans_test.go # utils/utils.go
attiasas
left a comment
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.
LGTM, make sure the PR description is formated
|
|
||
| callback := commonTests.MockProgressInitialization() | ||
| defer callback() | ||
| integration.InitScanTest(t, scangraph.GraphScanMinXrayVersion) |
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.
integration.InitScanTest should be the first line, skipping the test if needed
| err := securityTests.PlatformCli.Exec(args...) | ||
| assert.Error(t, err) |
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.
| err := securityTests.PlatformCli.Exec(args...) | |
| assert.Error(t, err) | |
| assert.Error(t, securityTests.PlatformCli.Exec(args...)) |
maybe you can also check if its the error you added (or error expected) and not random one
| args = []string{"scan", "--watch=my-watch", binariesPath} | ||
| } else { | ||
| args = []string{"scan", binariesPath, "--watch=my-watch"} |
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.
| args = []string{"scan", "--watch=my-watch", binariesPath} | |
| } else { | |
| args = []string{"scan", binariesPath, "--watch=my-watch"} | |
| args = []string{"scan", "--not-defined-flag=value", binariesPath} | |
| } else { | |
| args = []string{"scan", binariesPath, "--watches=my-watch"} |
change the flag name to make it more readable

devbranch.go vet ./....go fmt ./....The 'jf scan' command should be executed in the following manner: jf scan [command options] OR jf scan [command options] --spec=. [command options], it may lead that some flags are mistaken as arguments and are not going through the command's flags verifications. Therefore, if a wrong flag was provided in this manner - it will get skipped completely without informing the customer.
If the command was written in a different order: jf scan
I added a warning to indicate the wrong usage and what it may cause.
The indication comes in a Warning form and not error in order to not breaking customers.