Flaky unit tests

[StuAA78]

A couple of times recently, a unit test has failed during CI but passed when re-ran. We'll capture examples as and when they happen and record them here.

👍 Add a thumbs-up if a test is already logged but you encounter another example of it failing 👍

[StuAA78]

Test Bill Run Controller
  Generating a test bill run: POST /admin/test/{regimeId}/bill-runs
      74) creates a bill run with expected invoices and transactions

Failed on this assertion due to the invoices length being 4:

expect(invoices.length).to.equal(5)

[Cruikshanks]

Same as before, only this time

Expected 1 to equal specified value: 5

[Cruikshanks]

I think I know what it is. This test relies on a sleep() because we get a response immediately back but need to wait for the task in the background to complete. I just think sometimes the CI workflow needs to 'warmup' before it can create all the invoices fast enough.

We clearly need a better solution, or to rethink how we are testing this.

[StuAA78]

Good catch!

One possibility might be to replace:

      await sleep(1000)

      const billRun = await BillRunModel.query().findById(responsePayload.billRun.id)
      const invoices = await billRun.$relatedQuery('invoices')
      const transactions = await billRun.$relatedQuery('transactions')

with something like:

      let billRun
      let invoices
      let transactions
      let attempts = 0

      do {
        attempts += 1
        await sleep(500)
        billRun = await BillRunModel.query().findById(responsePayload.billRun.id)
        invoices = await billRun.$relatedQuery('invoices')
        transactions = await billRun.$relatedQuery('transactions')
      }
      while (attempts <= 3 && invoices.length !== 5 && transactions.length !== 15)

So it fetches the results every 500ms until we get the desired result, or until it's tried 3 times. So if it takes a little longer to get the results then that's fine, and if the correct result never arrives then we continue anyway so the assertions fail (whereas if we didn't limit it to 3 attempts the test would time out after the maximum of 2000ms -- not the end of the world but it would mean that the error we get isn't a true reflection of what failed).

[Cruikshanks]

Spot on @StuAA78 . The only alternate I had was to pull my finger out and write a test against the service directly, then just stub it out in the controller test so we no longer worry about waiting for a background process to complete. 🤷

[StuAA78]

  Generate a bill run summary: PATCH /v2/{regimeId}/bill-runs/{billRunId}/generate
    When the request is invalid
      because the summary has already been generated
        ✖ 156) returns error status 409

  156) Presroc Bill Runs controller Generate a bill run summary: PATCH /v2/{regimeId}/bill-runs/{billRunId}/generate When the request is invalid because the summary has already been generated returns error status 409:

      TRUNCATE TABLE "licences","regimes","authorised_systems","authorised_systems_regimes","sequence_counters","bill_runs","transactions","invoices","customers" RESTART IDENTITY - deadlock detected

      at Parser.parseErrorMessage (/home/runner/work/sroc-charging-module-api/sroc-charging-module-api/node_modules/pg-protocol/dist/parser.js:278:15)
      at Parser.handlePacket (/home/runner/work/sroc-charging-module-api/sroc-charging-module-api/node_modules/pg-protocol/dist/parser.js:126:29)
      at Parser.parse (/home/runner/work/sroc-charging-module-api/sroc-charging-module-api/node_modules/pg-protocol/dist/parser.js:39:38)
      at Socket.<anonymous> (/home/runner/work/sroc-charging-module-api/sroc-charging-module-api/node_modules/pg-protocol/dist/index.js:10:42)
      at Socket.emit (events.js:314:20)
      at addChunk (_stream_readable.js:298:12)
      at readableAddChunk (_stream_readable.js:273:9)
      at Socket.Readable.push (_stream_readable.js:214:10)
      at TCP.onStreamRead (internal/stream_base_commons.js:188:23)

[Cruikshanks]

Got another Expected 1 to equal specified value: 5 today

[Cruikshanks]

A new flaky test. We had another in the same area of 'move customers' today though we didn't manage to catch it. Might be worth revisting the tests when we can catch a break!

406) Move Customers To Exported Table service When called clears the customers table:

      Expected [
  CustomerModel {
    id: 'a478d5d0-c7fa-4181-a3ac-fce13b1885a1',
    regimeId: '8d15517b-1a16-4e7b-87a9-1490cf2661c3',
    region: 'A',
    customerReference: 'AB12345678',
    customerName: 'CUSTOMER_NAME',
    addressLine1: 'ADDRESS_LINE_1',
    addressLine2: 'ADDRESS_LINE_2',
    addressLine3: 'ADDRESS_LINE_3',
    addressLine4: 'ADDRESS_LINE_4',
    addressLine5: 'ADDRESS_LINE_5',
    addressLine6: 'ADDRESS_LINE_6',
    postcode: 'POSTCODE',
    createdAt: 2021-04-28T21:19:03.780Z,
    updatedAt: 2021-04-28T21:19:03.780Z,
    customerFileId: null
  }
] to be empty

      at /home/runner/work/sroc-charging-module-api/sroc-charging-module-api/test/services/move_customers_to_exported_table.service.test.js:64:28