Merge pull request #32766 from overleaf/bg-add-missing-unit-test-mocks

add missing mocks for @overleaf/metrics in unit tests

GitOrigin-RevId: 0903c3e26f88f92ef816a64f14ad053f159b31ed

services/web/test/unit/bootstrap.mjs

@@ -39,6 +39,13 @@ vi.mock('@overleaf/logger', async () => {
   }
 })
 
+// Mock metrics in unit tests, can be overridden
+vi.mock('@overleaf/metrics', () => {
+  return {
+    default: { prom: { Counter: vi.fn(), Histogram: vi.fn() } },
+  }
+})
+
 beforeEach(ctx => {
   // This function is a utility to duplicate the behaviour of passing `done` in place of `next` in an express route handler.
   ctx.rejectOnError = reject => {

services/web/test/unit/src/Authentication/AuthenticationController.test.mjs

@@ -10,8 +10,22 @@ const modulePath =
 
 const { ObjectId } = mongodb
 
+// We use vi.hoisted + vi.mock for @overleaf/metrics here because it is statically imported
+// by AuthenticationErrors.mjs at the top of this file. If we instead used vi.doMock inside
+// the beforeEach block, AuthenticationErrors.mjs would bind to the real unmocked metrics
+// module before the test setup even begins, causing errors when AuthenticationErrors calls it.
+const hoistedMocks = vi.hoisted(() => ({
+  metricsInc: vi.fn(),
+}))
+
+vi.mock('@overleaf/metrics', () => ({
+  default: {
+    inc: (...args) => hoistedMocks.metricsInc(...args),
+  },
+}))
+
 vi.mock(
-  '../../../../app/src/Features/Analytics/AnalyticsRegistrationSourceHelper.js',
+  '../../../../app/src/Features/Analytics/AnalyticsRegistrationSourceHelper.mjs',
   () => ({
     default: {
       clearInbound: vi.fn(),
@@ -53,6 +67,7 @@ describe('AuthenticationController', function () {
 
     vi.doMock(
       '../../../../app/src/Features/Authentication/AuthenticationErrors',
+      // () => ({ ...AuthenticationErrors })
       () => AuthenticationErrors
     )
 
@@ -93,9 +108,8 @@ describe('AuthenticationController', function () {
       }),
     }))
 
-    vi.doMock('@overleaf/metrics', () => ({
-      default: (ctx.Metrics = { inc: sinon.stub() }),
-    }))
+    ctx.Metrics = { inc: sinon.stub() }
+    hoistedMocks.metricsInc = ctx.Metrics.inc
 
     vi.doMock('../../../../app/src/Features/Security/LoginRateLimiter', () => ({
       default: (ctx.LoginRateLimiter = {
@@ -1557,7 +1571,7 @@ describe('AuthenticationController', function () {
     beforeEach(function (ctx) {
       ctx.userDetailsMap = new Map()
       ctx.logger.err = sinon.stub()
-      ctx.Metrics.inc = sinon.stub()
+      ctx.Metrics.inc.resetHistory()
     })
 
     describe('with valid credentials', function () {

services/web/test/unit/src/FileStore/FileStoreController.test.mjs

@@ -21,6 +21,9 @@ describe('FileStoreController', function () {
     ctx.HistoryManager = {
       promises: { requestBlobWithProjectId: sinon.stub() },
     }
+    ctx.Metrics = {
+      inc: sinon.stub(),
+    }
 
     vi.doMock('node:stream/promises', () => ctx.Stream)
 
@@ -36,6 +39,8 @@ describe('FileStoreController', function () {
       default: ctx.HistoryManager,
     }))
 
+    vi.doMock('@overleaf/metrics', () => ({ default: ctx.Metrics }))
+
     ctx.controller = (await import(MODULE_PATH)).default
     ctx.stream = {}
     ctx.projectId = '2k3j1lk3j21lk3j'

services/web/test/unit/src/Metadata/MetaController.test.mjs

@@ -32,6 +32,17 @@ describe('MetaController', function () {
       () => ({ default: {} })
     )
 
+    vi.doMock(
+      '../../../../app/src/Features/SplitTests/SplitTestHandler',
+      () => ({
+        default: {
+          promises: {
+            getAssignment: sinon.stub().resolves({}),
+          },
+        },
+      })
+    )
+
     ctx.MetadataController = (await import(modulePath)).default
   })
 

services/web/test/unit/src/Project/ProjectDetailsHandler.test.mjs

@@ -104,6 +104,8 @@ describe('ProjectDetailsHandler', function () {
       default: ctx.settings,
     }))
 
+    vi.doMock('@overleaf/metrics', () => ({ default: {} }))
+
     ctx.handler = (await import(MODULE_PATH)).default
   })
 

services/web/test/unit/src/SplitTests/SplitTestHandler.test.mjs

@@ -68,6 +68,9 @@ describe('SplitTestHandler', function () {
     ctx.SessionManager = {
       isUserLoggedIn: sinon.stub().returns(false),
     }
+    ctx.Metrics = {
+      inc: sinon.stub(),
+    }
 
     Features = {
       hasFeature: vi.fn().mockReturnValue(true),
@@ -129,6 +132,8 @@ describe('SplitTestHandler', function () {
       default: ctx.Settings,
     }))
 
+    vi.doMock('@overleaf/metrics', () => ({ default: ctx.Metrics }))
+
     ctx.SplitTestHandler = (await import(MODULE_PATH)).default
 
     ctx.req = new MockRequest(vi)

services/web/test/unit/src/Templates/TemplatesController.test.mjs

@@ -42,6 +42,8 @@ describe('TemplatesController', function () {
       })
     )
 
+    vi.doMock('@overleaf/metrics', () => ({ default: {} }))
+
     ctx.TemplatesController = (await import(modulePath)).default
     ctx.next = sinon.stub()
     ctx.req = {

services/web/test/unit/src/ThirdPartyDataStore/TpdsController.test.mjs

@@ -58,6 +58,9 @@ describe('TpdsController', function () {
         generateUniqueName: sinon.stub().resolves('unique'),
       },
     }
+    ctx.Metrics = {
+      inc: sinon.stub(),
+    }
 
     vi.doMock(
       '../../../../app/src/Features/ThirdPartyDataStore/TpdsUpdateHandler',
@@ -112,6 +115,8 @@ describe('TpdsController', function () {
       })
     )
 
+    vi.doMock('@overleaf/metrics', () => ({ default: ctx.Metrics }))
+
     ctx.TpdsController = (await import(MODULE_PATH)).default
 
     ctx.user_id = 'dsad29jlkjas'

services/web/test/unit/src/User/UserController.test.mjs

@@ -140,6 +140,10 @@ describe('UserController', function () {
       },
     }
 
+    ctx.Metrics = {
+      inc: sinon.stub(),
+    }
+
     vi.doMock(
       '../../../../app/src/Features/Analytics/AnalyticsManager',
       () => ({
@@ -236,6 +240,8 @@ describe('UserController', function () {
       default: ctx.Modules,
     }))
 
+    vi.doMock('@overleaf/metrics', () => ({ default: ctx.Metrics }))
+
     ctx.UserController = (await import(modulePath)).default
 
     ctx.res = {