Skip to content

Add route tests using supertest and node test runner #327

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

Draft
wants to merge 12 commits into
base: main
Choose a base branch
from

Conversation

hamishwillee
Copy link
Collaborator

@hamishwillee hamishwillee commented Jul 8, 2025

Want to be able to more easily check for errors when upgrading to new versions.

Currently run route tests (only) for all the models for list pages, detail pages, and the form get/post for delete, create, update. I.e. a fair chunk of cases.
This will also effectively test quite a bit of database interaction.

I'm still unsure about whether it is time to also add a guide on testing. But I am fairly sure this will be a good base for it.

There will minimally need to be a docs update for starting mongoose from the www file.
This is because mongoose can only take one connection. If we do this in the app as we were this means we can't tests. So this separates app creation and database setup.

This should go in with mdn/content#40247

This is part of fixing up mdn/content#38922

@hamishwillee hamishwillee requested a review from bsmth as a code owner July 8, 2025 03:02
@hamishwillee hamishwillee marked this pull request as draft July 8, 2025 03:02
windowMs: 1 * 10 * 1000, // 10 seconds
max: 10,
windowMs: 1 * 60 * 1000, // 10 seconds
max: 60,
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

So the tests can hammer out 50+ route tests very quickly.


// Display list of all Authors.
exports.author_list = asyncHandler(async (req, res, next) => {
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

These are the only substantial change - Express 5 does this automatically for you.

@@ -8,24 +8,28 @@
"scripts": {
"start": "node ./bin/www",
"devstart": "nodemon ./bin/www",
"serverstart": "DEBUG=express-locallibrary-tutorial:* npm run devstart"
"serverstart": "DEBUG=express-locallibrary-tutorial:* npm run devstart",
Copy link
Member

Choose a reason for hiding this comment

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

OK good, so if I run:

npm run serverstart
npm run test

I get all 50 tests passing, which is nice. One thing that might be interesting is checking if Node.js built-in test runner works with these kinds of tests. I think it's been there since node 18, and it's convenient because you don't need testing frameworks as deps.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@bsmth Good idea. Don't know how I missed that node has a test runner. I've migrated this to node: test (mostly us AI) and then tested my tests. All holds together and tests the same things.

This makes sense as the approach I would want learners to use, so is a better base for adding a test chapter in future.
Note thought, that writing such a chapter is not on my list of things to do for this round of work. I mostly just wanted to have a rapid way to test fixes/changes because I thought the issues with updating to express v5 would be ... bigger.

@hamishwillee hamishwillee changed the title Add route tests using supertest, chai, mocha Add route tests using supertest and node test runner Jul 11, 2025
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.

2 participants