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

DXCDT-417: Tests for test commands #694

Merged
merged 7 commits into from
Mar 31, 2023
Merged

Conversation

willvedd
Copy link
Contributor

@willvedd willvedd commented Mar 30, 2023

🔧 Changes

Adding integration tests for both the auth0 test token and auth0 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

  • All new/changed/fixed functionality is covered by tests (or N/A)
  • I have added documentation for all new/changed functionality (or N/A)

@@ -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
Copy link
Contributor Author

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.

Copy link
Contributor

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.

Copy link
Contributor Author

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.

Copy link
Contributor Author

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
Copy link
Contributor Author

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.

Copy link
Contributor

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?)

@willvedd willvedd marked this pull request as ready for review March 30, 2023 22:17
@willvedd willvedd requested a review from a team as a code owner March 30, 2023 22:17
@codecov-commenter
Copy link

Codecov Report

Patch coverage has no change and project coverage change: +1.76 🎉

Comparison is base (f08a170) 58.11% compared to head (405bfab) 59.87%.

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     
Impacted Files Coverage Δ
internal/cli/quickstarts.go 52.66% <ø> (+30.48%) ⬆️
internal/cli/test.go 41.63% <ø> (+19.65%) ⬆️

... 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.
📢 Do you have feedback about the report comment? Let us know in this issue.

@willvedd willvedd enabled auto-merge (squash) March 31, 2023 13:25
@willvedd willvedd merged commit bdc68bc into main Mar 31, 2023
@willvedd willvedd deleted the DXCDT-417-test-commands-tests branch March 31, 2023 13:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants