Skip to content

tests: add deep integration tests #224

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

Merged
merged 7 commits into from
Aug 26, 2023

Conversation

jankapunkt
Copy link
Member

Summary

This PR improves the integration tests to cover integrity of the model parameters.

Furthermore, compliance tests are added, where possible.

Finally, coverage is improved, if possible.

Linked issue(s)

#219

Involved parts of the project

Tests, grant types, handlers

OAuth2 standard

Compliance tests to deeply conform RFC 6749

Reproduction

checkout branch, run npm run test or npm run test:coverage

@jankapunkt jankapunkt self-assigned this Aug 17, 2023
@jankapunkt jankapunkt added in progress 👨‍💻 Currently in progress and is being worked on. security ❗ Address a security issue tests 🧪 Relates to tests labels Aug 17, 2023
@jankapunkt jankapunkt added this to the v5 milestone Aug 17, 2023
@jankapunkt jankapunkt linked an issue Aug 17, 2023 that may be closed by this pull request
};
const authorizationCode = 'long-authz-code-?';
const accessTokenDoc = {
accessToken: 'some-access-token-code-?',

Check failure

Code scanning / CodeQL

Hard-coded credentials

The hard-coded value "some-access-token-code-?" is used as [authorization header](1).
Copy link
Member Author

Choose a reason for hiding this comment

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

This is just tests, can be ignored

Copy link
Member

@jorenvandeweyer jorenvandeweyer left a comment

Choose a reason for hiding this comment

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

I already did a review. These tests are getting some meaning now!

Since it are mostly tests, I think you should already merge it.
(It takes a lot of time to review a PR as large as this one. Personally I prefer a lot of small PR's instead of 1 big PR 😅 )

@jankapunkt jankapunkt marked this pull request as ready for review August 26, 2023 07:00
@jankapunkt
Copy link
Member Author

@jorenvandeweyer you are right, let's merge and open a "Part II" PR for this

@jankapunkt jankapunkt merged commit c1fb9d4 into release-5.0.0 Aug 26, 2023
This was referenced Aug 26, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
in progress 👨‍💻 Currently in progress and is being worked on. security ❗ Address a security issue tests 🧪 Relates to tests
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Insufficient integration tests
2 participants