-
Notifications
You must be signed in to change notification settings - Fork 55
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
DXCDT-417: Tests for test
commands
#694
Conversation
@@ -269,7 +269,6 @@ func testTokenCmd(cli *cli) *cobra.Command { | |||
cmd.SetUsageTemplate(resourceUsageTemplate()) | |||
cmd.Flags().BoolVar(&cli.force, "force", false, "Skip confirmation.") | |||
cmd.Flags().BoolVar(&cli.json, "json", false, "Output in json format.") | |||
testAudience.IsRequired = true |
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.
Requiring the audience precluded the ability to test authentication in non-interactive mode.
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.
It looks like the initial change of toggling this to be required is in #221 otherwise it generated invalid tokens. Testing locally it still seems to be that dropping audience will generate an invalid token due to a missing payload.
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.
Let's talk about this later.
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.
Update: Turns out that this marks the audience flag required for both the test token
and test login
commands but it should only be requiring for test token
command. Will be fixing in a following commit.
tests: | ||
001 - test login of default app (partially successful): | ||
#Terminate early because command will otherwise wait for in-browser authorization stage | ||
command: timeout 2s auth0 test login $(./test/integration/scripts/get-default-app-id.sh) --no-input 2>&1 |
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.
Admittedly, looks odd. What's happening is running the auth0 test login
command but enforcing a 2s timeout and piping stderr into stdout. Otherwise, it will wait indefinitely for the user to authorize in the broswer. By terminating early, this test only verifies the first stage, but its better than nothing.
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.
Is timeout
a default command on MacOS? I don't appear to have it, maybe I need to install coreutils or something (I think that's fine though, maybe just needs a contributing note?)
Codecov ReportPatch coverage has no change and project coverage change:
Additional details and impacted files@@ Coverage Diff @@
## main #694 +/- ##
==========================================
+ Coverage 58.11% 59.87% +1.76%
==========================================
Files 89 89
Lines 11497 11493 -4
==========================================
+ Hits 6681 6882 +201
+ Misses 4363 4123 -240
- Partials 453 488 +35
... and 4 files with indirect coverage changes Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. ☔ View full report in Codecov by Sentry. |
🔧 Changes
Adding integration tests for both the
auth0 test token
andauth0 test login
commands. The bulk of this PR is orchestration of clients and client grants through scripts to emulate certain scenarios.🔬 Testing
Almost only testing additions.
📝 Checklist