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.

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

chore(fx-2514): enables jest eslint plugin #2858

Merged
merged 2 commits into from Dec 2, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 1 addition & 1 deletion .eslintignore
@@ -1,4 +1,4 @@
node_modules/*
build/*
scripts/*
**/__tests__/*

18 changes: 15 additions & 3 deletions .eslintrc.json
Expand Up @@ -6,12 +6,13 @@
"jasmine": true,
"es6": true
},
"plugins": ["promise", "@typescript-eslint"],
"plugins": ["promise", "@typescript-eslint", "jest"],
"extends": [
"plugin:promise/recommended",
"plugin:import/errors",
"plugin:import/typescript",
"plugin:@typescript-eslint/recommended"
"plugin:@typescript-eslint/recommended",
"plugin:jest/recommended"
],
"parser": "@typescript-eslint/parser",
"rules": {
Expand All @@ -37,7 +38,18 @@
],
"@typescript-eslint/no-use-before-define": 0,
"@typescript-eslint/explicit-module-boundary-types": 0,
"prefer-const": ["error", { "ignoreReadBeforeAssign": true }]
"prefer-const": ["error", { "ignoreReadBeforeAssign": true }],
"jest/expect-expect": [
"warn",
{
"assertFunctionNames": [
"expect",
"expectPromiseRejectionToMatch",
"assertStatusSupported",
"assertInvalidStatusFails"
]
}
]
},
"globals": {
"expect": false,
Expand Down
1 change: 1 addition & 0 deletions package.json
Expand Up @@ -137,6 +137,7 @@
"eslint": "6.7.2",
"eslint-import-resolver-typescript": "2.0.0",
"eslint-plugin-import": "2.18.2",
"eslint-plugin-jest": "^24.1.3",
"eslint-plugin-promise": "4.0.1",
"expect.js": "0.3.1",
"husky": "3.1.0",
Expand Down
2 changes: 1 addition & 1 deletion src/integration/__tests__/integration.test.ts
Expand Up @@ -13,7 +13,7 @@ describe("integration tests", () => {
mockFetch.mockReset()
})

it("It should bail for an unknown GET request", async () => {
it("should bail for an unknown GET request", async () => {
const response = await request(app).get("/")
expect(response.statusCode).toBe(400)
})
Expand Down
2 changes: 1 addition & 1 deletion src/integration/index.js
Expand Up @@ -24,7 +24,7 @@ const staging = metaphysics(METAPHYSICS_STAGING_ENDPOINT)
const production = metaphysics(METAPHYSICS_PRODUCTION_ENDPOINT)

describe("Integration specs", () => {
xdescribe("/artwork", () => {
describe.skip("/artwork", () => {
const query = `
query artwork($id: String!) {
artwork(id: $id) {
Expand Down
15 changes: 8 additions & 7 deletions src/lib/__tests__/apis/fetch.test.ts
Expand Up @@ -7,7 +7,7 @@ import fetch from "../../apis/fetch"
declare const expectPromiseRejectionToMatch: any

it("tries to parse the response when there is a String and resolves with it", async () => {
let reqResponse = {
const reqResponse = {
statusCode: 200,
body: `{ "foo": "bar" }`,
}
Expand All @@ -29,8 +29,8 @@ it("rejects request errors", async () => {
expectPromiseRejectionToMatch(fetch("foo/bar"), /bad/)
})

it("tries to parse the response when there is a String and resolves with it", async () => {
let reqResponse = {
it("tries to parse the response when there is a String and resolves with it (2)", async () => {
const reqResponse = {
statusCode: 200,
body: `{ not json }`,
}
Expand All @@ -42,8 +42,8 @@ it("tries to parse the response when there is a String and resolves with it", as
return expectPromiseRejectionToMatch(fetch("foo/bar"), /Unexpected token/)
})

it("tries to parse the response when there is a String and resolves with it", (done) => {
let reqResponse = {
it("tries to parse the response when there is a String and resolves with it (3)", () => {
const reqResponse = {
statusCode: 400,
request: {
uri: {
Expand All @@ -57,14 +57,15 @@ it("tries to parse the response when there is a String and resolves with it", (d
callback(null, reqResponse)
)

fetch("foo/bar").catch((error) => {
expect.assertions(3)

return fetch("foo/bar").catch((error) => {
expect(error.message).toEqual(
`http://api.artsy.net/api/v1/me - { "type": "other_error", "message": "undefined method \`[]' for nil:NilClass" }`
)
expect(error.statusCode).toEqual(400)
expect(error.body).toEqual(
`{ "type": "other_error", "message": "undefined method \`[]' for nil:NilClass" }`
)
done()
})
})
70 changes: 38 additions & 32 deletions src/lib/__tests__/cache.test.ts
@@ -1,4 +1,3 @@
/* eslint-disable promise/always-return */
import zlib from "zlib"
import config from "config"
import cache, { client, cacheKey } from "lib/cache"
Expand All @@ -23,36 +22,41 @@ function parseCacheResponse(data, cacheCompressionDisabled) {

describe("Cache with compression enabled", () => {
config.CACHE_COMPRESSION_DISABLED = true
expect(config.CACHE_COMPRESSION_DISABLED).toBe(true) // check that the config is mocked

// FIXME:
// eslint-disable-next-line jest/no-standalone-expect
expect(config.CACHE_COMPRESSION_DISABLED).toBe(true)

describe("when successfully connected to the cache", () => {
describe("#get", () => {
beforeEach(async () => await cache.set("get_foo", { bar: "baz" }))

it("parses the data and resolves the promise", () => {
return cache.get("get_foo").then((data) => {
expect(data.bar).toBe("baz")
})
it("parses the data and resolves the promise", async () => {
const data = await cache.get("get_foo")

expect(data.bar).toBe("baz")
})
})

describe("#delete", () => {
beforeEach(async () => await cache.set("get_foo", { bar: "baz" }))

it("deletes the data", async () => {
await cache.delete("get_foo")
expect.assertions(1)
try {
await cache.get("get_foo")
} catch (e) {
expect(e.message).toEqual("[Cache#get] Cache miss")
}

await cache.delete("get_foo")

await expect(cache.get("get_foo")).rejects.toThrow(
"[Cache#get] Cache miss"
)
})
})

describe("#set", () => {
describe("with a plain Object", () => {
it("sets the cache and includes a timestamp", async (done) => {
it("sets the cache and includes a timestamp", async () => {
expect.assertions(2)

await cache.set("set_foo", { bar: "baz" })

client.get(cacheKey("set_foo"), (_err, data) => {
Expand All @@ -63,13 +67,13 @@ describe("Cache with compression enabled", () => {

expect(parsed.bar).toBe("baz")
expect(typeof parsed.cached).toBe("number")

done()
})
})
})

it("with an Array it sets the cache and includes a timestamp", async (done) => {
it("with an Array it sets the cache and includes a timestamp", async () => {
expect.assertions(3)

await cache.set("set_bar", [{ baz: "qux" }])

client.get(cacheKey("set_bar"), (_err, data) => {
Expand All @@ -81,8 +85,6 @@ describe("Cache with compression enabled", () => {
expect(parsed.length).toBe(1)
expect(parsed[0].baz).toBe("qux")
expect(typeof parsed[0].cached).toBe("number")

done()
})
})
})
Expand All @@ -91,33 +93,39 @@ describe("Cache with compression enabled", () => {

describe("Cache with compression disabled", () => {
config.CACHE_COMPRESSION_DISABLED = false

// FIXME:
// eslint-disable-next-line jest/no-standalone-expect
expect(config.CACHE_COMPRESSION_DISABLED).toBe(false) // check that the config is mocked

describe("when successfully connected to the cache", () => {
describe("#get", () => {
beforeEach(async () => await cache.set("get_foo", { bar: "baz" }))

it("parses the data and resolves the promise", () => {
return cache.get("get_foo").then((data) => {
expect(data.bar).toBe("baz")
})
it("parses the data and resolves the promise", async () => {
const data = await cache.get("get_foo")

expect(data.bar).toBe("baz")
})
})

describe("#delete", () => {
beforeEach(async () => await cache.set("get_foo", { bar: "baz" }))

it("deletes the data", () => {
it("deletes the data", async () => {
cache.delete("get_foo")
return cache.get("get_foo").catch((e) => {
expect(e.message).toEqual("[Cache#get] Cache miss")
})

await expect(cache.get("get_foo")).rejects.toThrow(
"[Cache#get] Cache miss"
)
})
})

describe("#set", () => {
describe("with a plain Object", () => {
it("sets the cache and includes a timestamp", async (done) => {
it("sets the cache and includes a timestamp", async () => {
expect.assertions(2)

await cache.set("set_foo", { bar: "baz" })

client.get(cacheKey("set_foo"), (_err, data) => {
Expand All @@ -128,13 +136,13 @@ describe("Cache with compression disabled", () => {

expect(parsed.bar).toBe("baz")
expect(typeof parsed.cached).toBe("number")

done()
})
})
})

it("with an Array it sets the cache and includes a timestamp", async (done) => {
it("with an Array it sets the cache and includes a timestamp", async () => {
expect.assertions(3)

await cache.set("set_bar", [{ baz: "qux" }])

client.get(cacheKey("set_bar"), (_err, data) => {
Expand All @@ -146,8 +154,6 @@ describe("Cache with compression disabled", () => {
expect(parsed.length).toBe(1)
expect(parsed[0].baz).toBe("qux")
expect(typeof parsed[0].cached).toBe("number")

done()
})
})
})
Expand Down
24 changes: 12 additions & 12 deletions src/lib/__tests__/date.test.ts
Expand Up @@ -15,7 +15,7 @@ import {
} from "lib/date"

describe("date formatting", () => {
describe(timeRange, () => {
describe("timeRange", () => {
Copy link
Member

Choose a reason for hiding this comment

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

def think this form is a bit weird and unexpected and clever, but its also valid since it will read the function.name prop. Did this throw a warning / error?

Copy link
Member Author

Choose a reason for hiding this comment

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

Title must be a string (eslintjest/valid-title)

Copy link
Member

Choose a reason for hiding this comment

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

Non blocking, especially after all of this work was done, but i think that rule could be disabled. I can't remember where i read it but there's some interesting introspection benefits around passing the function in versus a string, but i also wonder about eslintjest setting this rule as a default -- in like a what do they know kind of way /shrug

Copy link
Member Author

Choose a reason for hiding this comment

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

I'd be curious to know what the benefits are, if you ever dig it up. I'm inclined to roll with the string form since it's just more consistent and the default behavior of the recommended settings. There's a bit of discussion I dug up here jest-community/eslint-plugin-jest#431 for context as well.

it("includes only one am or pm if within same am/pm", () => {
const period = timeRange(
"2022-12-30T08:00:00+00:00",
Expand Down Expand Up @@ -44,7 +44,7 @@ describe("date formatting", () => {
})
})

describe(datesAreSameDay, () => {
describe("datesAreSameDay", () => {
it("returns true if dates are the same day", () => {
const period = datesAreSameDay(
"2022-12-30T20:00:00+00:00",
Expand Down Expand Up @@ -82,7 +82,7 @@ describe("date formatting", () => {
})
})

describe(formattedStartDateTime, () => {
describe("formattedStartDateTime", () => {
const realNow = Date.now
beforeEach(() => {
Date.now = () => new Date("2018-01-30T03:24:00") as any
Expand Down Expand Up @@ -121,7 +121,7 @@ describe("date formatting", () => {
expect(period).toBe("Ended Dec 30, 2016")
})

it("includes 'Starts' when event starts in the future", () => {
it("includes 'Starts' when event starts in the future (2)", () => {
const period = formattedStartDateTime(
"2045-12-05T20:00:00+00:00",
"2050-12-30T17:00:00+00:00",
Expand All @@ -148,7 +148,7 @@ describe("date formatting", () => {
})
})

describe(formattedOpeningHours, () => {
describe("formattedOpeningHours", () => {
const realNow = Date.now
beforeEach(() => {
Date.now = () => new Date("2018-01-30T03:24:00") as any
Expand Down Expand Up @@ -185,7 +185,7 @@ describe("date formatting", () => {
})
})

describe(singleTime, () => {
describe("singleTime", () => {
it("includes hour using default UTC timezone", () => {
const period = singleTime("2018-12-05T20:00:00+00:00", "UTC")
expect(period).toBe("8:00pm UTC")
Expand All @@ -197,7 +197,7 @@ describe("date formatting", () => {
})
})

describe(singleDateTime, () => {
describe("singleDateTime", () => {
const realNow = Date.now
beforeEach(() => {
Date.now = () => new Date("2018-01-30T03:24:00") as any
Expand Down Expand Up @@ -225,7 +225,7 @@ describe("date formatting", () => {
})
})

describe(singleDate, () => {
describe("singleDate", () => {
const realNow = Date.now
beforeEach(() => {
Date.now = () => new Date("2018-01-30T03:24:00") as any
Expand All @@ -245,7 +245,7 @@ describe("date formatting", () => {
})
})

describe(singleDateWithDay, () => {
describe("singleDateWithDay", () => {
const realNow = Date.now
beforeEach(() => {
Date.now = () => new Date("2018-01-30T03:24:00") as any
Expand All @@ -265,7 +265,7 @@ describe("date formatting", () => {
})
})

describe(dateTimeRange, () => {
describe("dateTimeRange", () => {
const realNow = Date.now
beforeEach(() => {
Date.now = () => new Date("2018-01-30T03:24:00") as any
Expand Down Expand Up @@ -391,7 +391,7 @@ describe("date formatting", () => {
})
})

describe(dateRange, () => {
describe("dateRange", () => {
it("includes month day and both years when years are different years", () => {
const period = dateRange("2011-01-01", "2014-04-19", "UTC")
expect(period).toBe("Jan 1, 2011 – Apr 19, 2014")
Expand Down Expand Up @@ -422,7 +422,7 @@ describe("date formatting", () => {
})
})

describe(exhibitionStatus, () => {
describe("exhibitionStatus", () => {
let today: Moment = null as any

beforeEach(() => {
Expand Down