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.

Sign up for GitHub

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

Jump to bottom

feat: add unit tests for authentication middelware #2254

Merged
merged 4 commits into from
Nov 21, 2024
Merged

feat: add unit tests for authentication middelware #2254

merged 4 commits into from
Nov 21, 2024

Conversation

MayankBansal12
Copy link
Contributor

@MayankBansal12 MayankBansal12 commented Nov 17, 2024 •
edited by Achintya-Chatterjee
Loading

Date: 17-11-2024

Developer Name: Mayank Bansal

Issue Ticket Number

Description

  • Added unit test cases for authentication middleware inside authenticate.test.js
  • Due to a missing import statement was getting an error while running tests. Fixed the error by adding the missing config statement in three files.

Documentation Updated?

  • Yes
  • No

Under Feature Flag

  • Yes
  • No

Database Changes

  • Yes
  • No

Breaking Changes

  • Yes
  • No

Development Tested?

  • Yes
  • No

Screenshots

Test Coverage

Screenshot 1

added test cases

Screenshot 2

test coverage

@pankajjs
Copy link
Member

Are tests not working without importing config because it is a global variable?
@MayankBansal12

@MayankBansal12
Copy link
Contributor Author

@pankajjs I tried running it without the import, but I have been getting config is not defined error.

@pankajjs
Copy link
Member

@pankajjs I tried running it without the import, but I have been getting config is not defined error.

Then how is it running before? @MayankBansal12

@MayankBansal12
Copy link
Contributor Author

Then how is it running before? @MayankBansal12

I have been facing this error while importing the authMiddleware file
const authMiddleware = require("../../../middlewares/authenticate")

No other test files seems to using the same which could be one of the reasons why it was running before.

@pankajjs
Copy link
Member

Left few comments @MayankBansal12

@MayankBansal12
Copy link
Contributor Author

Left few comments @MayankBansal12

Will fix the issues mentioned

@MayankBansal12
Copy link
Contributor Author

@pankajjs Added a new commit making the changes mentioned above. Have a look.

pankajjs
pankajjs approved these changes Nov 19, 2024
View reviewed changes
Copy link
Member

@pankajjs pankajjs left a comment

Choose a reason for hiding this comment

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

LGTM ✅

@iamitprakash iamitprakash merged commit 20c9c1a into Real-Dev-Squad:develop Nov 21, 2024
3 checks passed
listiclehub1 pushed a commit to listiclehub1/website-backend that referenced this pull request Dec 2, 2024
@MayankBansal12 @listiclehub1
feat: add unit tests for authentication middelware (Real-Dev-Squad#2254)
c8f91e5 
* feat: add unit tests for authentication middelware

* fix: add missing config import statement

* feat: add more expectation for test cases

* fix: refactor set cookies and add assertions in stubs
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@iamitprakash iamitprakash iamitprakash approved these changes

@pankajjs pankajjs pankajjs approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Write Test for Authenticate Middleware
3 participants
@MayankBansal12 @pankajjs @iamitprakash