-
Notifications
You must be signed in to change notification settings - Fork 1
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
Feat/testing approaches #61
base: main
Are you sure you want to change the base?
Conversation
G0maa
commented
Oct 19, 2024
- E2E, Integration (kind of) & unit testing (also kind of).
✅ Deploy Preview for disworse ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
session({ | ||
secret: configService.getOrThrow<string>("SESSION_SECRET"), | ||
resave: false, | ||
saveUninitialized: false, | ||
cookie: { | ||
maxAge: configService.getOrThrow<number>("COOKIE_MAX_AGE"), | ||
httpOnly: true, | ||
}, | ||
store: new RedisStore({ | ||
client: redisClient, | ||
}), | ||
}), |
Check warning
Code scanning / CodeQL
Clear text transmission of sensitive cookie Medium test
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix AI 19 days ago
To fix the problem, we need to ensure that the session cookie is only transmitted over HTTPS by setting the secure
attribute to true
. This change should be made in the session configuration object within the beforeAll
method of the TestManager
class.
- Locate the session configuration object in the
beforeAll
method. - Add the
secure: true
attribute to thecookie
object within the session configuration.
-
Copy modified line R43
@@ -42,2 +42,3 @@ | ||
httpOnly: true, | ||
secure: true, | ||
}, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Codecov ReportAll modified and coverable lines are covered by tests ✅
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why are that fields like created_at
nullable?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@bassiounix iirc this is the type of the result of find
by Drizzle.
|
||
await testManager.app.init(); | ||
|
||
// TODO: Apply Database migrations/seeders. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Where are the db stuff?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm... I can't recall why I didn't add them, I can work on them this weekend, but I prefer that we merge this anway.
export class TestManager { | ||
// biome-ignore lint/suspicious/noExplicitAny: it is any. | ||
public httpServer: any; | ||
public app: INestApplication; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
INestApplication<NestExpressApplication>
could be more expressive. not mandatory.
@@ -0,0 +1,47 @@ | |||
import { ContextIdFactory } from "@nestjs/core"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Unused imports!!!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Test coverage is low, not many functions are being tested.
Otherwise .. LGTM.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A worker process has failed to exit gracefully and has been force exited. This is likely caused by tests leaking due to improper teardown. Try running with --detectOpenHandles to find leaks. Active timers can also cause this, ensure that .unref() was called on them.
appears when trying to run jest
.
Also test:e2e
shows a similar message:
Force exiting Jest: Have you considered using `--detectOpenHandles` to detect async operations that kept running after all tests finished?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@bassiounix
The intention of this branch is not to test. The intention is to show developer working on Disworse HOW to test. With examples on Unit, E2E, and ((kind of)) integration tests.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also on the Jest error, I never managed to fix it, I usually silence it with --forceExit
.