Skip to content
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.

Sign up for GitHub

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

Jump to bottom

test: convert tests to jest #929

Merged
merged 2 commits into from
Jul 16, 2020
Merged

test: convert tests to jest #929

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
032f016
test: convert tests to jest
djskinner Jul 13, 2020
1abf8f7
Merge branch 'next' into convert-tests-to-jest
bengourley Jul 16, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 5 additions & 1 deletion jest.config.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,11 @@ module.exports = {
testsForPackage('plugin-server-*'),
testsForPackage('plugin-strip-project-root'),
testsForPackage('plugin-intercept'),
testsForPackage('plugin-node-unhandled-rejection')
testsForPackage('plugin-node-unhandled-rejection'),
testsForPackage('plugin-node-in-project'),
testsForPackage('plugin-node-device'),
testsForPackage('plugin-node-surrounding-code'),
testsForPackage('plugin-node-uncaught-exception')
]
}
]
Expand Down
2 changes: 1 addition & 1 deletion packages/core/types/event.d.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ export interface Stackframe {
method?: string
lineNumber?: number
columnNumber?: number
code?: object
code?: Record<string, string>
inProject?: boolean
}

Expand Down
7 changes: 1 addition & 6 deletions packages/plugin-node-device/package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,14 +14,9 @@
"files": [
"*.js"
],
"scripts": {
"test": "nyc --reporter=lcov -- jasmine '!(node_modules)/**/*.test.js'"
},
"author": "Bugsnag",
"license": "MIT",
"devDependencies": {
"@bugsnag/core": "^7.2.0",
"jasmine": "^3.1.0",
"nyc": "^12.0.2"
"@bugsnag/core": "^7.2.0"
}
}
11 changes: 5 additions & 6 deletions ...es/plugin-node-device/test/device.test.js → ...es/plugin-node-device/test/device.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,8 +1,6 @@
const { describe, it, expect } = global
import plugin from '../device'
import Client from '@bugsnag/core/client'

const plugin = require('../device')

const Client = require('@bugsnag/core/client')
const schema = {
...require('@bugsnag/core/config').schema,
hostname: {
Expand All @@ -29,9 +27,10 @@ describe('plugin: node device', () => {
expect(payload.events[0].device.freeMemory).toBeGreaterThan(0)
expect(payload.events[0].device.totalMemory).toBeGreaterThan(payload.events[0].device.freeMemory)
expect(payload.events[0].device.runtimeVersions).toBeDefined()
expect(payload.events[0].device.runtimeVersions.node).toEqual(process.versions.node)
expect(payload.events[0].device.runtimeVersions && payload.events[0].device.runtimeVersions.node).toEqual(process.versions.node)
done()
}
},
sendSession: () => {}
}))
client.notify(new Error('noooo'))
})
Expand Down
7 changes: 1 addition & 6 deletions packages/plugin-node-in-project/package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,14 +14,9 @@
"files": [
"*.js"
],
"scripts": {
"test": "nyc --reporter=lcov -- jasmine '!(node_modules)/**/*.test.js'"
},
"author": "Bugsnag",
"license": "MIT",
"devDependencies": {
"@bugsnag/core": "^7.2.0",
"jasmine": "^3.1.0",
"nyc": "^12.0.2"
"@bugsnag/core": "^7.2.0"
}
}
12 changes: 5 additions & 7 deletions ...n-node-in-project/test/in-project.test.js → ...n-node-in-project/test/in-project.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,10 +1,8 @@
const { describe, it, expect } = global

const plugin = require('../')
const { join } = require('path')
const Event = require('@bugsnag/core/event')
const Client = require('@bugsnag/core/client')
const { schema } = require('@bugsnag/core/config')
import plugin from '../'
import { join } from 'path'
import Event from '@bugsnag/core/event'
import Client from '@bugsnag/core/client'
import { schema } from '@bugsnag/core/config'

describe('plugin: node in project', () => {
it('should mark stackframes as "inProject" if it is a descendent of the "projectRoot"', done => {
Expand Down
7 changes: 1 addition & 6 deletions packages/plugin-node-surrounding-code/package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,18 +14,13 @@
"files": [
"*.js"
],
"scripts": {
"test": "nyc --reporter=lcov -- jasmine '!(node_modules)/**/*.test.js'"
},
"author": "Bugsnag",
"license": "MIT",
"dependencies": {
"byline": "^5.0.0",
"pump": "^3.0.0"
},
"devDependencies": {
"@bugsnag/core": "^7.2.0",
"jasmine": "^3.1.0",
"nyc": "^12.0.2"
"@bugsnag/core": "^7.2.0"
}
}
33 changes: 16 additions & 17 deletions ...unding-code/test/surrounding-code.test.js → ...unding-code/test/surrounding-code.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,38 +1,37 @@
const { describe, it, expect } = global
/* eslint-disable @typescript-eslint/no-non-null-assertion */
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I wonder if it's worth disabling this rule across all test files as its usage is often legitimate and useful.

Often there are cases where a previous assertion asserts the existence of something. Unfortunately the typings of expect aren't sufficient to avail the TS compiler of such knowledge (perhaps it is possible to do with better typings that use assertion signatures - I'm not sure). Even without the benefit of previous assertions, it seems legitimate to me to make assumptions about the existence of things because the test would just fail otherwise.

FWIW we agreed that it would be OK disable this rule in the dashboard-js project though we haven't got round to doing it yet. That might be because in that project we are more relaxed about overriding rules on a per-line or entire file basis where judged appropriate.

On a related note I've seen a few places in the tests where the optional chaining ?. operator would be useful. Though currently jest doesn't like it when used. I think we might need to update our TypeScript/jest/babel dependencies if we want to make use of that one.

Copy link
Contributor

Choose a reason for hiding this comment

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

I think it's a reasonable thing to assert. Since the implementation is JS, the type system doesn't guarantee that such things are genuinely non-null.

Maybe updating the test dev dependencies should come in via another PR. The ?. and also ! are nice operators for terse TS so it would be good to have access to them.

import fs from 'fs'
import plugin from '../'
import { join } from 'path'
import Event from '@bugsnag/core/event'
import Client from '@bugsnag/core/client'

const fs = require('fs')
let createReadStreamCount = 0
const originalReadStream = fs.createReadStream
fs.createReadStream = function () {
createReadStreamCount++
return originalReadStream.apply(fs, arguments)
return originalReadStream.apply(fs, arguments as any)
}

const plugin = require('../')
const { join } = require('path')
const Event = require('@bugsnag/core/event')
const Client = require('@bugsnag/core/client')

describe('plugin: node surrounding code', () => {
it('should load code successfully for stackframes whose files exist', done => {
const client = new Client({ apiKey: 'api_key' }, undefined, [plugin])

client._setDelivery(client => ({
sendEvent: (payload) => {
const evt = payload.events[0]
expect(Object.keys(evt.errors[0].stacktrace[0].code))
expect(Object.keys(evt.errors[0].stacktrace[0].code!))
.toEqual(['19', '20', '21', '22', '23', '24', '25'])
expect(evt.errors[0].stacktrace[0].code['22'])
expect(evt.errors[0].stacktrace[0].code!['22'])
.toBe(' if (cb) this.on(\'finish\', () => cb(this.output()))')

expect(Object.keys(evt.errors[0].stacktrace[1].code))
expect(Object.keys(evt.errors[0].stacktrace[1].code!))
.toEqual(['28', '29', '30', '31', '32', '33', '34'])
expect(evt.errors[0].stacktrace[1].code['31'])
expect(evt.errors[0].stacktrace[1].code!['31'])
.toBe(' return nextLevelUp()')

expect(Object.keys(evt.errors[0].stacktrace[2].code))
expect(Object.keys(evt.errors[0].stacktrace[2].code!))
.toEqual(['115', '116', '117', '118', '119', '120', '121'])
expect(evt.errors[0].stacktrace[2].code['118'])
expect(evt.errors[0].stacktrace[2].code!['118'])
.toBe(' \'Ķ\': \'k\', \'Ļ\': \'L\', \'Ņ\': \'N\', \'Ū\': \'u\'')

done()
Expand Down Expand Up @@ -133,7 +132,7 @@ describe('plugin: node surrounding code', () => {
client._setDelivery(client => ({
sendEvent: (payload) => {
const endCount = createReadStreamCount
expect(endCount - startCount).toBe(1)
expect(endCount - startCount).toBe(0)
payload.events[0].errors[0].stacktrace.forEach(stackframe => {
expect(stackframe.code).toEqual({
1: '// this is just some arbitrary (but real) javascript for testing, taken from',
Expand Down Expand Up @@ -197,8 +196,8 @@ describe('plugin: node surrounding code', () => {
client._setDelivery(client => ({
sendEvent: (payload) => {
payload.events[0].errors[0].stacktrace.forEach(stackframe => {
Object.keys(stackframe.code).forEach(key => {
expect(stackframe.code[key].length <= 200).toBe(true)
Object.keys(stackframe.code!).forEach(key => {
expect(stackframe.code![key].length <= 200).toBe(true)
})
})
done()
Expand Down
7 changes: 1 addition & 6 deletions packages/plugin-node-uncaught-exception/package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,14 +14,9 @@
"files": [
"*.js"
],
"scripts": {
"test": "nyc --reporter=lcov -- jasmine '!(node_modules)/**/*.test.js'"
},
"author": "Bugsnag",
"license": "MIT",
"devDependencies": {
"@bugsnag/core": "^7.2.0",
"jasmine": "^3.1.0",
"nyc": "^12.0.2"
"@bugsnag/core": "^7.2.0"
}
}
29 changes: 14 additions & 15 deletions ...exception/test/uncaught-exception.test.js → ...exception/test/uncaught-exception.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,8 +1,7 @@
const { describe, it, expect } = global

const Client = require('@bugsnag/core/client')
const schema = require('@bugsnag/core/config').schema
const plugin = require('../')
import Client from '@bugsnag/core/client'
import { schema } from '@bugsnag/core/config'
import plugin from '../'
import EventWithInternals from '@bugsnag/core/event'

describe('plugin: node uncaught exception handler', () => {
it('should listen to the process#uncaughtException event', () => {
Expand Down Expand Up @@ -37,7 +36,7 @@ describe('plugin: node uncaught exception handler', () => {
it('should call the configured onUncaughtException callback', done => {
const c = new Client({
apiKey: 'api_key',
onUncaughtException: (err, event) => {
onUncaughtException: (err: Error, event: EventWithInternals) => {
expect(err.message).toBe('never gonna catch me')
expect(event.errors[0].errorMessage).toBe('never gonna catch me')
expect(event._handledState.unhandled).toBe(true)
Expand All @@ -50,22 +49,22 @@ describe('plugin: node uncaught exception handler', () => {
}, {
...schema,
onUncaughtException: {
validate: val => typeof val === 'function',
validate: (val: unknown) => typeof val === 'function',
message: 'should be a function',
defaultValue: () => {}
}
})
c._setDelivery(client => ({
sendEvent: (...args) => args[args.length - 1](),
sendSession: (...args) => args[args.length - 1]()
sendEvent: (payload, cb) => cb(),
sendSession: (payload, cb) => cb()
}))
process.listeners('uncaughtException')[1](new Error('never gonna catch me'))
process.listeners('uncaughtException')[0](new Error('never gonna catch me'))
})

it('should tolerate delivery errors', done => {
const c = new Client({
apiKey: 'api_key',
onUncaughtException: (err, event) => {
onUncaughtException: (err: Error, event: EventWithInternals) => {
expect(err.message).toBe('never gonna catch me')
expect(event.errors[0].errorMessage).toBe('never gonna catch me')
expect(event._handledState.unhandled).toBe(true)
Expand All @@ -78,15 +77,15 @@ describe('plugin: node uncaught exception handler', () => {
}, {
...schema,
onUncaughtException: {
validate: val => typeof val === 'function',
validate: (val: unknown) => typeof val === 'function',
message: 'should be a function',
defaultValue: () => {}
}
})
c._setDelivery(client => ({
sendEvent: (...args) => args[args.length - 1](new Error('failed')),
sendSession: (...args) => args[args.length - 1]()
sendEvent: (payload, cb) => cb(new Error('failed')),
sendSession: (payload, cb) => cb()
}))
process.listeners('uncaughtException')[1](new Error('never gonna catch me'))
process.listeners('uncaughtException')[0](new Error('never gonna catch me'))
})
})
6 changes: 5 additions & 1 deletion tsconfig.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,10 @@
"packages/plugin-browser-request",
"packages/plugin-simple-throttle",
"packages/plugin-intercept",
"packages/plugin-node-unhandled-rejection"
"packages/plugin-node-unhandled-rejection",
"packages/plugin-node-in-project",
"packages/plugin-node-device",
"packages/plugin-node-surrounding-code",
"packages/plugin-node-uncaught-exception"
]
}