Addes auth security tests for nutri help#160
Addes auth security tests for nutri help#160Priyanshutyagiji wants to merge 3 commits intoGopher-Industries:masterfrom
Conversation
elanlaw1206
left a comment
There was a problem hiding this comment.
HI Priyanshu,
Thanks for adding auth/security test coverage , the intent and scope look good.
A few things to address before merge:
-
package.json: the "test" script is replaced from the existing Mocha wildcard to a single Jest test file. This may stop other tests from running and affect CI coverage. Suggest keeping the current "test" command and adding a separate script (e.g. "test:auth"), or handling Jest migration in a dedicated PR.
-
auth.test.js: BASE_URL is hardcoded to http://localhost:80, which is fragile for CI and local runs. Please switch to process.env.BASE_URL with a fallback, or ideally use supertest(app) by importing the Express app directly.
-
auth.test.js: several assertions allow 404 for protected endpoints. Allowing 404 can hide missing routes and create false positives in security tests. Suggest narrowing expected status codes to 401/403 (and 400 where applicable).
Once these are updated, I’m happy to re-review and approve.
King Hei
|
Hi Priyanshu, Thanks for the update, appreciate the work on this 👍 Since this PR includes changes to core files (e.g., package.json and security-related logic), I’m taking a safety-first approach to ensure it doesn’t affect the main runtime or CI behaviour. Could you please share the test evidence showing everything still works as expected? CI run link (green checks), and The test command you ran locally with a brief output/result Once confirmed and all checks remain green, I’m happy to approve and move this forward. Thanks! King Hei |

No description provided.