Skip to content

Add tests for iOS loop push notifications and websockets#8419

Open
je-l wants to merge 8 commits intonightscout:devfrom
je-l:extend-api-tests
Open

Add tests for iOS loop push notifications and websockets#8419
je-l wants to merge 8 commits intonightscout:devfrom
je-l:extend-api-tests

Conversation

@je-l
Copy link
Copy Markdown

@je-l je-l commented Jan 15, 2026

Adds high-level integration tests related to iOS loop push notifications and websocket API. This should help with e.g. dependency updates and to avoid introducing regression bugs when making changes.

Statement coverage increased from 63.8% to 65.4%.
Branch coverage from 51.0% to 53.0%.

@AndyLow91
Copy link
Copy Markdown
Member

Thanks for the PR. The additional coverage looks useful, but I think this needs revision before merge.

A few issues should be addressed first:

  • loopnotifications.test.js appears order-dependent around API_SECRET. The suite creates the app using process.env.API_SECRET, but authenticates using a hard-coded hash for a different secret. That makes the test fragile depending on prior test state.
  • Both new suites start real Nightscout server instances but do not fully clean them up. Please add proper teardown for the server/bus and any other test resources.
  • The fake APNs server uses a fixed port, which makes the test more brittle and can cause avoidable conflicts.

If you can make the new tests fully self-contained and deterministic, I’d be happy to take another look.

je-l added 3 commits March 17, 2026 20:27
Duplicated behavior between websocket.test.js and
websocket.shape-handling.test.js.
Also use dynamic port for the fake APNs backend.

See nightscout#8419 (comment)
@je-l
Copy link
Copy Markdown
Author

je-l commented Mar 17, 2026

Pushed fixes to all three comments, the teardown looks like this now:

after(function(done) {
  inst.server.close();
  inst.ctx.bus.teardown();

  delete process.env.LOOP_APNS_KEY;
  delete process.env.LOOP_APNS_KEY_ID;
  delete process.env.LOOP_DEVELOPER_TEAM_ID;
  delete process.env.ENABLE;

  apn.Provider = OriginalApnProviderClass;

  guardedDrop(inst.ctx.profile(), err => {
    if (err) return done(err)

    fakeAPNServer.close(done);
  });
});

I removed websocket.test.js, the overlap is high with websocket.shape-handling.test.js.
Interestingly, the old websocket tests didn't pass. I think it shouldn't be a regression bug, but I will look a bit more later and create a new issue if needed. For me this PR is ready to merge.

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