Skip to content

Commit

Permalink
Add eslint-plugin-jest and jest rules; fix tests that violate eslint …
Browse files Browse the repository at this point in the history
…rules (#883)
  • Loading branch information
silesky authored Jul 13, 2023
1 parent 9ff3fe4 commit 1309919
Show file tree
Hide file tree
Showing 16 changed files with 173 additions and 148 deletions.
9 changes: 9 additions & 0 deletions .eslintrc.js
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,8 @@ module.exports = {
'plugin:@typescript-eslint/recommended',
// Handle prettier rules through eslint https://github.com/prettier/eslint-plugin-prettier/blob/master/eslint-plugin-prettier.js#L65
'plugin:prettier/recommended',
// add special jest rules https://github.com/jest-community/eslint-plugin-jest#rules
'plugin:jest/recommended',
],
rules: {
'@typescript-eslint/explicit-function-return-type': 'off',
Expand All @@ -50,6 +52,13 @@ module.exports = {
'@typescript-eslint/require-await': 'error',
'@typescript-eslint/no-empty-function': 'off',
'@typescript-eslint/ban-types': 'off', // TODO: turn on
/** jest */
'jest/valid-title': 'off', // allow functions to be used as describe titles
'jest/no-conditional-expect': 'off', // best practice, but TODO
'jest/no-alias-methods': 'off', // best practice, but TODO
'jest/expect-expect': 'off', // sometimes we compose assertion functions
'jest/no-disabled-tests': process.env.CI ? 'error' : 'off', // fail CI if tests are disabled, but do not interfere with local development
'jest/no-focused-tests': process.env.CI ? 'error' : 'off',
},
overrides: [
{
Expand Down
2 changes: 1 addition & 1 deletion .husky/pre-push
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
#!/bin/sh
. "$(dirname "$0")/_/husky.sh"

yarn test
CI=true yarn test --colors --silent
1 change: 1 addition & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@
"eslint": "^8.14.0",
"eslint-config-prettier": "^8.5.0",
"eslint-plugin-import": "^2.27.5",
"eslint-plugin-jest": "^27.2.2",
"eslint-plugin-prettier": "^4.0.0",
"express": "^4.18.2",
"get-monorepo-packages": "^1.2.0",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -141,14 +141,14 @@ describe('Pre-initialization', () => {

describe('Promise API', () => {
describe('.then', () => {
test('.then should be called on success', (done) => {
test('.then should be called on success', () => {
const ajsBrowser = AnalyticsBrowser.load({ writeKey: 'abc' })
const newPromise = ajsBrowser.then(([analytics, context]) => {
expect(analytics).toBeInstanceOf<typeof Analytics>(Analytics)
expect(context).toBeInstanceOf<typeof Context>(Context)
done()
})
expect(newPromise).toBeInstanceOf<typeof Promise>(Promise)
return newPromise
})

it('.then should pass to the next .then', async () => {
Expand All @@ -160,15 +160,12 @@ describe('Pre-initialization', () => {
})

describe('.catch', () => {
it('should be capable of handling errors if using promise syntax', () => {
it('should be capable of handling errors if using promise syntax', async () => {
browserLoadSpy.mockImplementationOnce((): any => Promise.reject(errMsg))

const ajsBrowser = AnalyticsBrowser.load({ writeKey: 'abc' })
const newPromise = ajsBrowser.catch((reason) => {
expect(reason).toBe(errMsg)
})
expect(newPromise).toBeInstanceOf(Promise)
expect.assertions(2)
await expect(() =>
AnalyticsBrowser.load({ writeKey: 'abc' })
).rejects.toEqual(errMsg)
})
})

Expand Down
6 changes: 3 additions & 3 deletions packages/browser/src/browser/__tests__/integration.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,11 +18,11 @@ import { getCDN, setGlobalCDNUrl } from '../../lib/parse-cdn'
import { clearAjsBrowserStorage } from '../../test-helpers/browser-storage'
import { parseFetchCall } from '../../test-helpers/fetch-parse'
import { ActionDestination } from '../../plugins/remote-loader'
import { UADataValues } from '../../lib/client-hints/interfaces'
import {
highEntropyTestData,
lowEntropyTestData,
} from '../../lib/client-hints/__tests__/index.test'
import { UADataValues } from '../../lib/client-hints/interfaces'
} from '../../test-helpers/fixtures/client-hints'

let fetchCalls: ReturnType<typeof parseFetchCall>[] = []

Expand Down Expand Up @@ -1083,7 +1083,7 @@ describe('Options', () => {
expect(integrationEvent.timestamp()).toBeInstanceOf(Date)
})

it('converts iso strings to dates be default', async () => {
it('does not convert iso strings to dates be default if disableAutoISOConversion is false', async () => {
const initOptions: InitOptions = { disableAutoISOConversion: false }
const [analytics] = await AnalyticsBrowser.load(
{
Expand Down
3 changes: 2 additions & 1 deletion packages/browser/src/core/buffer/__tests__/index.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -204,7 +204,7 @@ describe('AnalyticsBuffered', () => {

test('Will ignore the .then if there is a catch block', () => {
const thenCb = jest.fn()
new AnalyticsBuffered(() => {
const p = new AnalyticsBuffered(() => {
return Promise.reject('nope') as any
})
.then(() => {
Expand All @@ -215,6 +215,7 @@ describe('AnalyticsBuffered', () => {
})
expect(thenCb).not.toBeCalled()
expect.assertions(2)
return p
})
})
})
Expand Down
1 change: 1 addition & 0 deletions packages/browser/src/core/events/__tests__/index.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -331,6 +331,7 @@ describe('Event Factory', () => {
})
})

// eslint-disable-next-line jest/no-disabled-tests
describe.skip('anonymousId', () => {
// TODO: the code should be fixed so that these tests can pass -- this eventFactory does not seem to handle these edge cases well.
// When an event is dispatched, there are four places anonymousId can live: event.anonymousId, event.options.anonymousId, event.context.anonymousId, and the user object / localStorage.
Expand Down
30 changes: 5 additions & 25 deletions packages/browser/src/lib/client-hints/__tests__/index.test.ts
Original file line number Diff line number Diff line change
@@ -1,29 +1,9 @@
import { clientHints } from '..'
import { UADataValues, UALowEntropyJSON } from '../interfaces'

export const lowEntropyTestData: UALowEntropyJSON = {
brands: [
{
brand: 'Google Chrome',
version: '113',
},
{
brand: 'Chromium',
version: '113',
},
{
brand: 'Not-A.Brand',
version: '24',
},
],
mobile: false,
platform: 'macOS',
}

export const highEntropyTestData: UADataValues = {
architecture: 'x86',
bitness: '64',
}
import {
highEntropyTestData,
lowEntropyTestData,
} from '../../../test-helpers/fixtures/client-hints'
import { UADataValues } from '../interfaces'

describe('Client Hints API', () => {
beforeEach(() => {
Expand Down

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -5,11 +5,11 @@ import { Analytics } from '../../../core/analytics'
import { Plugin } from '../../../core/plugin'
import { pageEnrichment } from '../../page-enrichment'
import cookie from 'js-cookie'
import { UADataValues } from '../../../lib/client-hints/interfaces'
import {
highEntropyTestData,
lowEntropyTestData,
} from '../../../lib/client-hints/__tests__/index.test'
import { UADataValues } from '../../../lib/client-hints/interfaces'
} from '../../../test-helpers/fixtures/client-hints'

jest.mock('unfetch', () => {
return jest.fn()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -371,10 +371,6 @@ describe('before loading', () => {
assert(!object.context.amp)
})

describe('failed initializations', () => {
it.skip('should add failedInitializations as part of _metadata object if this.analytics.failedInitilizations is not empty', () => {})
})

describe('unbundling', () => {
it('should add a list of bundled integrations', () => {
normalize(analytics, object, options, {
Expand Down
28 changes: 28 additions & 0 deletions packages/browser/src/test-helpers/fixtures/client-hints.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
import {
UADataValues,
UALowEntropyJSON,
} from '../../lib/client-hints/interfaces'

export const lowEntropyTestData: UALowEntropyJSON = {
brands: [
{
brand: 'Google Chrome',
version: '113',
},
{
brand: 'Chromium',
version: '113',
},
{
brand: 'Not-A.Brand',
version: '24',
},
],
mobile: false,
platform: 'macOS',
}

export const highEntropyTestData: UADataValues = {
architecture: 'x86',
bitness: '64',
}
25 changes: 0 additions & 25 deletions packages/core/src/queue/__tests__/event-queue.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -648,31 +648,6 @@ describe('dispatchSingle', () => {
expect(eq.queue.length).toBe(0)
})

it.skip('records delivery metrics', async () => {
// Skip because we don't support metrics atm
const eq = new TestEventQueue()
const ctx = await eq.dispatchSingle(
new TestCtx({
type: 'track',
})
)

expect(ctx.logs().map((l) => l.message)).toMatchInlineSnapshot(`
Array [
"Dispatching",
"Delivered",
]
`)

expect(ctx.stats.metrics.map((m) => m.metric)).toMatchInlineSnapshot(`
Array [
"message_dispatched",
"message_delivered",
"delivered",
]
`)
})

test('retries retriable cancelations', async () => {
// make sure all backoffs return immediatelly
jest.spyOn(timer, 'backoff').mockImplementationOnce(() => 100)
Expand Down
2 changes: 1 addition & 1 deletion packages/node/src/__tests__/http-integration.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,7 @@ describe('Method Smoke Tests', () => {
],
}
`)
expect(scope.isDone())
expect(scope.isDone()).toBeTruthy()
})
})

Expand Down
6 changes: 3 additions & 3 deletions packages/node/src/__tests__/integration.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -338,7 +338,7 @@ describe('ready', () => {
expect(analytics['_queue'].plugins.length).toBeGreaterThan(0)
})

it.skip('should not reject if a plugin fails registration during initialization?', async () => {
// TODO: we should test the unhappy path
})
it.todo(
'should not reject if a plugin fails registration during initialization?'
)
})
Loading

0 comments on commit 1309919

Please sign in to comment.