Skip to content

Commit

Permalink
fix(tests): fix flakiness, again…
Browse files Browse the repository at this point in the history
Turns out the real reason was that we had `begin`/`commit`s creating transactions in our kitchen sink SQL queries. Therefore the changes were not being rolled back appropriately. Turns out `begin`/`commit` does not compose.

The fix would involve simply removing `begin` and `commit` from `resources/kitchen-sink-data.sql` and `resources/kitchen-sink-schema.sql`, however we made a couple of other good changes in this commit trying to fix the problem.
  • Loading branch information
calebmer committed Oct 9, 2016
1 parent d8bd1bc commit ed6065d
Show file tree
Hide file tree
Showing 9 changed files with 112 additions and 151 deletions.
5 changes: 2 additions & 3 deletions resources/kitchen-sink-data.sql
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
begin;
-- We don’t have a `begin`/`commit` in here to let the users of this query
-- control the data lifecycle.

insert into c.person (id, name, email, about, created_at) values
(1, 'John Smith', '[email protected]', null, null),
Expand Down Expand Up @@ -31,5 +32,3 @@ insert into c.compound_key (person_id_1, person_id_2, extra) values
(2, 5, true),
(2, 3, null),
(4, 4, false);

commit;
9 changes: 3 additions & 6 deletions resources/kitchen-sink-schema.sql
Original file line number Diff line number Diff line change
@@ -1,8 +1,7 @@
begin;
-- We don’t have a `begin`/`commit` in here to let the users of this query
-- control the data lifecycle.

drop schema if exists a cascade;
drop schema if exists b cascade;
drop schema if exists c cascade;
drop schema if exists a, b, c cascade;

create schema a;
create schema b;
Expand Down Expand Up @@ -108,5 +107,3 @@ create table b.types (
-- b text not null,
-- unique (a, b)
-- );

commit;
17 changes: 8 additions & 9 deletions src/postgraphql/__tests__/postgraphqlIntegration-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import { resolve as resolvePath } from 'path'
import { readFile, readdirSync } from 'fs'
import { GraphQLSchema, printSchema, graphql } from 'graphql'
import { Inventory, Context } from '../../interface'
import getTestPGClient from '../../postgres/__tests__/fixtures/getTestPGClient'
import withPGClient from '../../postgres/__tests__/fixtures/withPGClient'
import { introspectDatabase } from '../../postgres/introspection'
import addPGCatalogToInventory from '../../postgres/inventory/addPGCatalogToInventory'
import { createPGContext } from '../../postgres/inventory/pgContext'
Expand All @@ -20,8 +20,7 @@ const kitchenSinkData = new Promise((resolve, reject) => {
*/
let schema1, schema2

beforeAll(async () => {
const client = await getTestPGClient()
beforeAll(withPGClient(async client => {
const catalog = await introspectDatabase(client, ['a', 'b', 'c'])

const inventory1 = new Inventory()
Expand All @@ -31,7 +30,7 @@ beforeAll(async () => {
const inventory2 = new Inventory()
addPGCatalogToInventory(inventory2, catalog, { renameIdToRowId: true })
schema2 = createGraphqlSchema(inventory2, { nodeIdFieldName: 'id' })
})
}))

test('schema', async () => {
expect(printSchema(schema1)).toMatchSnapshot()
Expand All @@ -41,20 +40,20 @@ test('schema', async () => {
const queriesDir = resolvePath(__dirname, 'fixtures/queries')

for (const file of readdirSync(queriesDir)) {
test(`query ${file}`, async () => {
const query = await (new Promise((resolve, reject) => {
test(`query ${file}`, withPGClient(async client => {
const query = await new Promise((resolve, reject) => {
readFile(resolvePath(queriesDir, file), (error, data) => {
if (error) reject(error)
else resolve(data.toString())
})
}))
})

const client = await getTestPGClient()
await client.query(await kitchenSinkData)

const context = createPGContext(client)

const result = await graphql(schema1, query, null, context)

expect(result).toMatchSnapshot()
})
}))
}
67 changes: 0 additions & 67 deletions src/postgres/__tests__/fixtures/getTestPGClient.js

This file was deleted.

46 changes: 46 additions & 0 deletions src/postgres/__tests__/fixtures/withPGClient.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
import { resolve as resolvePath } from 'path'
import { readFile } from 'fs'
import { Pool, Client } from 'pg'
import minify = require('pg-minify')

const pool = new Pool({
database: 'postgraphql_test',
port: 5432,
max: 15,
idleTimeoutMillis: 500,
})

const kitchenSinkSchema = new Promise<string>((resolve, reject) => {
readFile(resolvePath(__dirname, '../../../../resources/kitchen-sink-schema.sql'), (error, data) => {
if (error) reject(error)
else resolve(minify(data.toString()))
})
})

/**
* Takes a function implementation of a test, and provides it a Postgres
* client. The client will be connected from the pool at the start of the test,
* and released back at the end. All changes will be rolled back.
*/
export default function withPGClient (fn: (client: Client) => void | Promise<void>) {
return async () => {
// Connect a client from our pool and begin a transaction.
const client = await pool.connect()
await client.query('begin')
await client.query(await kitchenSinkSchema)

// Mock the query function.
client.query = jest.fn(client.query)

// Try to run our test, if it fails we still want to cleanup the client.
try {
await fn(client)
}
// Always rollback our changes and release the client, even if the test
// fails.
finally {
await client.query('rollback')
client.release()
}
}
}
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import getTestPGClient from '../../__tests__/fixtures/getTestPGClient'
import withPGClient from '../../__tests__/fixtures/withPGClient'
import createKitchenSinkPGSchema from '../../__tests__/fixtures/createKitchenSinkPGSchema'
import introspectDatabase from '../introspectDatabase'

Expand Down Expand Up @@ -89,8 +89,7 @@ const format = catalog => ({
})),
})

test('will get everything needed in an introspection', async () => {
const client = await getTestPGClient()
test('will get everything needed in an introspection', withPGClient(async client => {
expect(format(await introspectDatabase(client, ['a', 'b', 'c']))).toMatchSnapshot()
expect(format(await introspectDatabase(client, ['a']))).toMatchSnapshot()
})
}))
Loading

0 comments on commit ed6065d

Please sign in to comment.