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

Fix recording of tests when using a separate authentication api instance #322

Merged
merged 6 commits into from
Dec 5, 2023

Conversation

ewanharris
Copy link
Contributor

🔧 Changes

Improves the coverage of the authentication client when running make test-e2e and skips tests that ultimately are not testable in that test scenario. Also updates the contributing documentation to help with creating the required applications.

📚 References

🔬 Testing

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

@codecov-commenter
Copy link

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (30c8dec) 95.18% compared to head (c3a9a9a) 95.10%.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #322      +/-   ##
==========================================
- Coverage   95.18%   95.10%   -0.08%     
==========================================
  Files          46       46              
  Lines        8956     8956              
==========================================
- Hits         8525     8518       -7     
- Misses        329      334       +5     
- Partials      102      104       +2     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

CONTRIBUTING.md Outdated

Then create a local `.env` file in the `authentication` folder with the following settings:

* `AUTH0_DOMAIN`: The **Domain** of the M2M app
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
* `AUTH0_DOMAIN`: The **Domain** of the M2M app
* `AUTH0_DOMAIN`: The **Domain** of the Auth0 tenant

Comment on lines +34 to +35
mgmtClientID = os.Getenv("AUTH0_CLIENT_ID")
mgmtClientSecret = os.Getenv("AUTH0_CLIENT_SECRET")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should these be AUTH0_MGMT_CLIENT_ID and AUTH0_MGMT_CLIENT_SECRET to more clearly distinguish them from their auth counterparts?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd left these as matching what's used for the management tests to make it easier to reuse them (some folks might opt to set environment variables themselves instead of the .env file)

@ewanharris ewanharris enabled auto-merge (squash) December 5, 2023 10:38
@ewanharris ewanharris merged commit b238963 into main Dec 5, 2023
7 checks passed
@ewanharris ewanharris deleted the test/SDK-4359-auth-e2e branch December 5, 2023 10:38
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.

3 participants