Skip to content

test: add test helpers and fix suite bugs#407

Merged
leowla merged 4 commits into
masterfrom
vps-19-test-coverage
Jun 5, 2026
Merged

test: add test helpers and fix suite bugs#407
leowla merged 4 commits into
masterfrom
vps-19-test-coverage

Conversation

@leowla

@leowla leowla commented May 20, 2026

Copy link
Copy Markdown
Member

Issue

  • the Express error handler had wrong arity (2 params instead of 4),
  • nextFunction mock was never cleared between tests causing cross-test contamination
  • deprecated Mongoose connection options were passed
  • the DELETE scenario test had no assertion that the access list entry was removed.
  • user.js had a pre-existing bug where Object.entries was called on a Mongoose query result array, passing whole documents as IDs instead of extracting ._id

Solution

Fix bugs and dedupe Mongoose connection code in test suite

Risk

Low. We can rely on the test suite!!!
Potential risk with user.js fix which corrects broken behaviour (user assignment was passing document objects instead of IDs to the query)

Checklist

  • Acceptance criteria met
  • Wiki documentation is written and up to date
  • Integration tests written and passing

@leowla leowla requested review from harbassan and hazikchaudhry May 20, 2026 08:28
@leowla leowla self-assigned this May 20, 2026
@leowla leowla added the backend label May 20, 2026
@linear

linear Bot commented May 20, 2026

Copy link
Copy Markdown

VPS-19

@harbassan harbassan left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

awesome, that should make implementing testing much much easier 😄. approving for the sake of progress on this, but I think it might be a good idea to rename mongoSetup.js given that it also has the express setup helper within it.

@harbassan harbassan requested a review from K1mmyn May 30, 2026 11:08
leowla added 3 commits May 31, 2026 17:49
- Fix Express error handler arity in imageApi (2 params → 4; Express
  requires exactly 4 to recognise error-handling middleware)
- Reset nextFunction mock between tests in scenarioAuth to prevent
  call-count bleed across test cases
- Remove deprecated useNewUrlParser/useUnifiedTopology options from
  sceneDao that generated driver warnings
- Add missing Access document deletion assertion to scenario DELETE test
- Replace Object.entries(array).map(([_, doc]) => doc) in user.js with
  .map(doc => doc._id), which passed Mongoose documents where ObjectIds
  were expected
- Configure no-unused-vars argsIgnorePattern for underscore-prefixed
  params; remove now-redundant eslint-disable comment in user.js
Add src/test/mongoSetup.js with two composable functions:
useMongoMemoryServer() registers beforeAll/afterEach/afterAll lifecycle
hooks for MongoMemoryServer; useExpressServer(configureApp) registers
beforeAll/afterAll for an Express server and returns a ctx object whose
port is populated at test time.

Replace ~15 lines of duplicated boilerplate in all five test files with
two calls at the top of each describe block. Also fixes a latent teardown
race: the original server.close(async () => {...}) passed an async
callback that Jest never awaited; the helper uses a Promise-wrapped close
so mongoose.disconnect and mongoServer.stop are properly sequenced.
@leowla leowla force-pushed the vps-19-test-coverage branch from 98f45c1 to c3c0608 Compare May 31, 2026 06:08

@K1mmyn K1mmyn left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

lgtm maintains all functionality and you improved the code overall

nice docs

@leowla leowla changed the title test: fix test suite bugs test: add test helpers and fix suite bugs Jun 5, 2026
@leowla leowla merged commit 7ce73d1 into master Jun 5, 2026
3 checks passed
@leowla leowla removed the request for review from hazikchaudhry June 5, 2026 05:51
@leowla leowla deleted the vps-19-test-coverage branch June 9, 2026 11:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants