DEV Community: Aleksei Tsikov

Testing-library: avoid these mistakes in async tests

Aleksei Tsikov — Sun, 21 Nov 2021 16:58:28 +0000

Testing is a crucial part of any large application development. The more code you write, the more tests you want to add to make sure all the parts still work together as expected. Here in Revolut, a lot of things happen behind our mobile super-app. We have a lot of backoffice apps with complex logic, and need to be sure nothing is broken when new features are added.

Sometimes, tests start to unexpectedly fail even if no changes were made to the business logic. It may happen after e.g. you updated some underlying library, made changes to the network layer, etc. Good and stable tests should still reliably assert component output against the given input, no matter what happens at the lower levels. Another even worse case is when tests still pass even when the component logic got broken.

In this article, I would like to show a few common mistakes that could lead to such issues, how to fix these, and how to make your tests stable and predictable. Initially, I picked this topic for our internal Revolut knowledge share session, but I feel like it could be helpful for a broader audience.

These and a few more examples could be found in this repository.

Await with sync methods
Async methods without await
Side-effects inside waitFor

Await with sync methods

Simple asynchronous request

This is the most common mistake I'm running into while refactoring code. Let's say, you have a simple component that fetches and shows user info. For the sake of simplicity, our API will only capitalize the given user id and return it as a user name. I'm also using react-query-alike hooks, but not the library itself, to make things more transparent:

const getUser = async (id: string): Promise<string> =>
  id[0].toUpperCase().concat(id.slice(1))

const useUserQuery = (id: string | null) => {
  const [data, setData] = useState<string | null>(null)
  useEffect(() => {
    if (!id) {
      setData(null)
      return
    }
    getUser(id).then(setData)
  }, [id])
  return data
}

const UserView = ({ id }: { id: string | null }) => {
  const data = useUserQuery(id)
  if (data === null) return <div>Loading...</div>
  return <>{data}</>
}

We want to write a test for it, so we are rendering our component with React Testing Library (RTL for short) and asserting that an expected string is visible to our user:

it('should render user info', async () => {
  await render(<UserView id="bob" />)
  expect(screen.getByText('Bob')).not.toBeNull()
})

So far, this test works perfectly well.

Complex asyncronous request

Later, a new requirement comes in to display not only a user but also their partner name. Easy-peasy! Let's just change our fetch function a little bit, and then update an assertion.
In getUser, we will now wait for two consecutive requests and only then return the aggregated data:

const getUser = async (id: string): Promise<string> => {
  const user = await getUser(id)
  const partner = await (user[0] === 'A'
    ? getUser('charlie')
    : getUser('daisy'))
  return `${user} and ${partner}`
}

And let's update our test as well:

it('should render user info', async () => {
  await render(<UserView id="bob" />)
  expect(screen.getByText('Alice and Charlie')).not.toBeNull()
})

Our changes made perfect sense, but suddenly our test will start to fail with "Unable to find an element with the text: Alice and Charlie". Oh-oh! But we didn't change any representation logic, and even the query hook is the same. Also, RTL output shows "Loading..." text in our DOM, though it looks like we are awaiting for render to complete in the very first line of our test.

Explanation

Alright, let's find out what's going on here. render is a synchronous function, but await is designed to work with asynchronous ones. What's going on when render is awaited? Well, MDN is very clear about it:

If the value of the expression following the await operator is not a Promise, it's converted to a resolved Promise.

In our test, when we are calling render with await, JavaScript implicitly wraps the result into a promise and waits for it to be settled. Meanwhile, we already have another pending promise scheduled in the fetch function. By the time implicit awaited promise is resolved, our fetch is resolved as well, as it was scheduled earlier. So we have the correct output on the screen.

But after the latest changes, our fetch function waits for the two consecutive promises, thus data is not fully ready after implicit render promise is resolved. In fact, even in the first green test, react warned us about something going wrong with an "act warning", because actual update after fetch promise was resolved happened outside of RTL's act wrappers:

Warning: An update to UserAndPartnerView inside a test was not wrapped in act(...).

When testing, code that causes React state updates should be wrapped into act(...):

act(() => {
  /* fire events that update state */
});
/* assert on the output */

This ensures that you're testing the behavior the user would see in the browser. Learn more at https://reactjs.org/link/wrap-tests-with-act
    at UserAndPartnerView (./common-testing-library-mistakes/src/a-await-sync-methods/UserAndPartnerView.tsx:3:38)

Now, that we know what exactly caused the error, let's update our test. RTL provides a set of methods that return promises and are resolved when an element is found. This is the perfect case to use one of these:

it('should render user info', async () => {
  render(<UserView id="bob" />)
  expect(await screen.findByText('Alice and Charlie')).not.toBeNull()
})

Now, we don't care how many requests happen while the component is being rendered. findByText will wait for the given text to appear in the DOM.

Conclusion

You should never await for syncronous functions, and render in particular. Use the proper asyncronous utils instead:

expect(await screen.findByText('some text')).not.toBe(null)
// or
await waitFor(() => {
  expect(screen.getByText('some text')).not.toBe(null)
})

Async methods without await

Positive case

Let's face the truth: JavaScript gives us hundreds of ways to shoot in a leg. And while async/await syntax is very convenient, it is very easy to write a call that returns a promise without an await in front of it.

Let's see how this could cause issues in our tests. I will be writing a test for the same UserView component we created in a previous example:

it('should render user info', async () => {
  render(<UserView id="alice" />)
  waitFor(() => {
    expect(screen.getByText('Alice')).not.toBeNull()
  })
})

This test passes, and everything looks good. Indeed, for a user with an id "alice", our request should return the name "Alice".

False-positive case

Now, let's see if our test fails when we pass the incorrect id

it('should render user info', async () => {
  render(<UserView id="bob" />)
  waitFor(() => {
    expect(screen.getByText('Alice')).not.toBeNull()
  })
})

Oops, it's still passing. But "bob"'s name should be Bob, not Alice.

Explanation

The reason is the missing await before asyncronous waitFor call. Asyncronous method call will always return a promise, which will not be awaited on its own. Jest simply calls this line and finishes the test. No assertions fail, so the test is green. But if we add await in front of waitFor, the test will fail as expected:

it('should render user info', async () => {
  render(<UserView id="bob" />)
  await waitFor(() => {
    expect(screen.getByText('Alice')).not.toBeNull()
  })
})

Unable to find an element with the text: Alice.

Ignored nodes: comments, <script />, <style />
<body>
  <div>
    Bob
  </div>
</body>

Conclusion

Never forget to await for async functions or return promises from the test (jest will wait for this promise to be resolved in this case). Otherwise, you may end up running tests that always pass.

Side-effects inside waitFor

How waitFor works

First of all, let's recall what is waitFor. It's an async RTL utility that accepts a callback and returns a promise. This promise is resolved as soon as the callback doesn't throw, or is rejected in a given timeout (one second by default). waitFor will call the callback a few times, either on DOM changes or simply with an interval.

Now, keeping all that in mind, let's see how side-effects inside waitFor could lead to unexpected test behavior.

Green test

Here, we have a component that renders a list of user transactions. Each list entry could be clicked to reveal more details.

const TransactionDetails = ({
  description,
  merchant,
}: {
  description?: string | null
  merchant?: string | null
}) => (
  <ul>
    {description && <li>Description: {description}</li>}
    {merchant && <li>Merchant: {merchant}</li>}
  </ul>
)

const Transactions = () => {
  const [selectedTransactionId, setSelectedTransactionId] = useState<
    string | null
  >(null)

  const transactions = useTransactionsQuery()
  if (transactions === null) return <div>Loading...</div>

  return (
    <ul>
      {transactions.map(tx => (
        <li
          key={tx.id}
          onClick={() =>
            setSelectedTransactionId(
              selectedTransactionId === tx.id ? null : tx.id,
            )
          }
        >
          <div>Id: {tx.id}</div>
          {selectedTransactionId === tx.id && (
            <TransactionDetails description={tx.description} />
          )}
        </li>
      ))}
    </ul>
  )
}

And the test to cover this logic:

it('should render transaction details', async () => {
  render(<Transactions />)

  await waitFor(() => {
    fireEvent.click(screen.getByText('Id: one'))
    expect(screen.getByText('Description: Coffee')).not.toBeNull()
  })
})

As the transactions list appears only after the request is done, we can't simply call screen.getByText('Id: one') because it will throw due to missing "Id: one" text. To avoid it, we put all the code inside waitFor which will retry on error. So we are waiting for the list entry to appear, clicking on it and asserting that description appears.

Hanging test

Now, let's add a bit more logic and fetch the transaction details as soon as it is clicked. Again, as in the very first example, we should not significantly change the test as the component basically stays the same. So we only want to add another assertion to make sure that the details were indeed fetched.

We will slightly change the component to fetch more data when one of the transactions is selected, and to pass fetched merchant name inside TransactionDetails. When nothing is selected, useTransactionDetailsQuery returns null, and the request is only triggered when an id is passed.

const TransactionsWithDetails = () => {
  // ...

  const transactions = useTransactionsQuery()
  const details = useTransactionDetailsQuery(selectedTransactionId)

  // ...
          <div>Id: {tx.id}</div>
          {selectedTransactionId === tx.id && (
            <TransactionDetails
              description={tx.description}
              merchant={details?.merchant}
            />
          )}
  // ...
}

First, the user sees the list of transactions. Then, as soon as one is clicked, details are fetched and shown.

As was mentioned earlier, in our test we will only add another assertion to check that merchant name from the details is rendered:

it('should render transaction details', async () => {
  render(<TransactionsWithDetails />)

  await waitFor(() => {
    fireEvent.click(screen.getByText('Id: one'))
    expect(screen.getByText('Description: Coffee')).not.toBeNull()
    expect(screen.getByText('Merchant: Mega Mall one')).not.toBeNull()
  })
})

When we run our updated test, we could notice that the test runner hangs. And while it's relatively easy to find the problem when we deal with a single test, it's a pain to find such a broken one in another few hundred.

Explanation

Let's figure out what is happenning here. waitFor is triggered multiple times because at least one of the assertions fails. Let's go through the sequence of calls, where each list entry represents the next waitFor call:

fireEvent.click(screen.getByText('Id: one')) fails as transactions list is not yet fetched, and "Id: one" text is not at the screen.
"Id: one" is present and clicked, but now expect(screen.getByText('Merchant: Mega Mall one')).not.toBeNull() fails as details are not yet fetched.
The above successful fireEvent.click triggered a DOM mutation, so waitFor executes the callback once again. fireEvent.click is triggered once again, closing the transaction description, and expect(screen.getByText('Description: Coffee')).not.toBeNull() fails.

As at the third call fireEvent.click caused another DOM mutation, we stuck in 2-3 loop. Transaction details are being opened and closed over and over again with no chance for the details request to complete and to render all the needed info.

The fix for the issue is very straightforward: we simply need to move our side-effect (fireEvent.click) out of waitFor.

it('should render transaction details', async () => {
  render(<TransactionsWithDetails />)

  const transaction = await screen.findByText('Id: one'))
  fireEvent.click(transaction)

  await waitFor(() => {
    expect(screen.getByText('Description: Coffee')).not.toBeNull()
    expect(screen.getByText('Merchant: Mega Mall one')).not.toBeNull()
  })
})

Conclusion

As waitFor is non-deterministic and you cannot say for sure how many times it will be called, you should never run side-effects inside it. Instead, wait for certain elements to appear on the screen, and trigger side-effects synchronously.

How to avoid these issues

The simplest way to stop making these mistakes is to add eslint-plugin-testing-library to your eslint.

testing-library/await-async-utils makes sure you are awaiting for async methods like waitFor and waitForElementToBeRemoved
testing-library/await-async-query protects you against missing awaits with asyncronous findBy... and findAllBy...
testing-library/no-wait-for-side-effects doesn't allow you to write side-effects inside waitFor

The only thing it doesn't catch is await render, but works perfectly well for everything else.

Summary

Debugging asynchronous tests could be pretty difficult, but you could simply make your tests more failure-proof avoiding the mistakes I described above.

Unfortunately, most of the "common mistakes" articles only highlight bad practices, without providing a detailed explanation. I hope I closed this gap, and my post gave you enough details on why the above mistakes should be avoided.

And make sure you didn't miss rather old but still relevant Kent C. Dodds' Common mistakes with React Testing Library where more issues are described.

Advanced TypeScript: reinventing lodash.get

Aleksei Tsikov — Sun, 05 Sep 2021 11:07:45 +0000

As a part of a backoffice team in a financial organisation, I have to deal with a lot of complex data structures: customer personal data, transactions, you name it. Sometimes you need to present a value that lies deep inside a data object. To make life simpler, I could use
lodash.get which allows me to access a value by its path, and avoid endless obj.foo && obj.foo.bar conditions (though it's not a case anymore after optional chaining had landed).

What is wrong with this approach?

While _.get works perfectly well in runtime, it comes with a huge drawback when used with TypeScript: in a majority of cases, it cannot infer value type, which could lead to various issues during refactoring.

Let's say a server sends us data with a customer's address stored this way



type Address = {
  postCode: string
  street: [string, string | undefined]
}

type UserInfo = {
  address: Address
  previousAddress?: Address
}

const data: UserInfo = {
  address: {
    postCode: "SW1P 3PA",
    street: ["20 Deans Yd", undefined]
  }
}

And now we want to render it



import { get } from 'lodash'

type Props = {
  user: UserInfo
}
export const Address = ({ user }: Props) => (
  <div>{get(user, 'address.street').filter(Boolean).join(', ')}</div>
)

Later, at some point we would like to refactor this data structure and use slightly different address representation



type Address = {
  postCode: string
  street: {
    line1: string
    line2?: string
  }
}

Since _.get always returns any for path strings, TypeScript will not notice any issues, while code will throw in runtime, because filter method doesn't exist on our new Address object.

Adding types

Since v4.1, which was released in Nov 2020, TypeScript has a feature called Template Literal Types. It allows us to build templates out of literals and other types. Let's see how it could help us.

Parsing dot-separated paths

For the most common scenario, we want TypeScript to correctly infer value type by a given path inside an object. For the above example, we want to know a type for address.street to be able to early notice the issue with an updated data structure. I will also use Conditional Types. If you are not familiar with conditional types, just think of it as a simple ternary operator, that tells you if one type matches another.

First of all, let's check if our path is actually a set of dot separated fields



type IsDotSeparated<T extends string> = T extends `${string}.${string}`
  ? true
  : false

type A = IsDotSeparated<'address.street'> // true
type B = IsDotSeparated<'address'> // false

Looks simple, right? But how could we extract the actual key?
Here comes a magic keyword infer which will help us to get parts of a string



type GetLeft<T extends string> = T extends `${infer Left}.${string}`
  ? Left
  : undefined

type A = GetLeft<'address.street'> // 'address'
type B = GetLeft<'address'> // undefined

And now, it's time to add our object type. Let's start with a simple case



type GetFieldType<Obj, Path> = Path extends `${infer Left}.${string}`
  ? Left extends keyof Obj
    ? Obj[Left]
    : undefined
  : Path extends keyof Obj
    ? Obj[Path]
    : undefined


type A = GetFieldType<UserInfo, 'address.street'> // Address, for now we only taking a left part of a path
type B = GetFieldType<UserInfo, 'address'> // Address
type C = GetFieldType<UserInfo, 'street'> // undefined

First, we are checking if our passed path matches string.string template. If so, we are taking its left part, checking if it exists in the keys of our object, and returning a field type.

If the path didn't match a template, it might be a simple key. For this case, we are doing similar checks and returning field type, or undefined as a fallback.

Adding a recursion

Ok, we got the correct type for a top-level field. But it gives us a little value. Let's improve our utility type and go down the path to the required value.

We are going to:

Find a top-level key
Get a value by a given key
Remove this key from our path
Repeat the whole process for our resolved value and the rest of the key until there's no Left.Right match



export type GetFieldType<Obj, Path> =
  Path extends `${infer Left}.${infer Right}`
    ? Left extends keyof Obj
      ? GetFieldType<Obj[Left], Right>
      : undefined
    : Path extends keyof Obj
      ? Obj[Path]
      : undefined

type A = GetFieldType<UserInfo, 'address.street'> // { line1: string; line2?: string | undefined; }
type B = GetFieldType<UserInfo, 'address'> // Address
type C = GetFieldType<UserInfo, 'street'> // undefined

Perfect! Looks like that's exactly what we wanted.

Handling optional properties

Well, there's still a case we need to take into account. UserInfo type has an optional previousAddress field. Let's try to get previousAddress.street type



type A = GetFieldType<UserInfo, 'previousAddress.street'> // undefined

Ouch! But in case previousAddress is set, street will definitely not be undefined.

Let's figure out what happens here. Since previousAddress is optional, its type is Address | undefined (I assume you have strictNullChecks turned on). Obviously, street doesn't exist on undefined, so there is no way to infer a correct type.

We need to improve our GetField. To retrieve a correct type, we need to remove undefined. However, we need to preserve it on the final type, as the field is optional, and the value could indeed be undefined.

We could achieve this with two TypeScript built-in utility types:
Exclude which removes types from a given union, and Extract which extracts types from a given union, or returns never in case there are no matches.



export type GetFieldType<Obj, Path> = Path extends `${infer Left}.${infer Right}`
  ? Left extends keyof Obj
    ? GetFieldType<Exclude<Obj[Left], undefined>, Right> | Extract<Obj[Left], undefined>
    : undefined
  : Path extends keyof Obj
    ? Obj[Path]
    : undefined

// { line1: string; line2?: string | undefined; } | undefined
type A = GetFieldType<UserInfo, 'previousAddress.street'>

When undefined is present in the value type, | Extract<> adds it to the result. Otherwise, Extract returns never which is simply ignored.

And this is it! Now we have a nice utility type that will help to make our code much safer.

Implementing a utility function

Now that we taught TypeScript how to get correct value types, let's add some runtime logic. We want our function to split a dot-separated path into parts, and reduce this list to get the final value. The function itself is really simple.



export function getValue<
  TData,
  TPath extends string,
  TDefault = GetFieldType<TData, TPath>
>(
  data: TData,
  path: TPath,
  defaultValue?: TDefault
): GetFieldType<TData, TPath> | TDefault {
  const value = path
    .split('.')
    .reduce<GetFieldType<TData, TPath>>(
      (value, key) => (value as any)?.[key],
      data as any
    );

  return value !== undefined ? value : (defaultValue as TDefault);
}

We have to add some ugly as any type castings because

intermediate values could indeed be of any type;
Array.reduce expects the initial value to be of the same type as a result. However, it's not the case here. Also, despite having three generic type parameters, we don't need to provide any types there. As all generics are mapped to function parameters, TypeScript infers these upon the function call from the actual values.

Making component type-safe

Let's revisit our component. In the initial implementation, we used lodash.get which didn't raise an error for a mismatched type. But with our new getValue, TypeScript will immediately start to complain

Adding support for [] notation

_.get supports keys like list[0].foo. Let's implement the same in our type. Again, literal template types will help us to get index keys from square brackets. I will not go step by step this time and instead will post the final type and some comments below.



type GetIndexedField<T, K> = K extends keyof T 
  ? T[K]
  : K extends `${number}`
    ? '0' extends keyof T
      ? undefined
      : number extends keyof T
        ? T[number]
        : undefined
    : undefined

type FieldWithPossiblyUndefined<T, Key> =
  | GetFieldType<Exclude<T, undefined>, Key>
  | Extract<T, undefined>

type IndexedFieldWithPossiblyUndefined<T, Key> =
  | GetIndexedField<Exclude<T, undefined>, Key>
  | Extract<T, undefined>

export type GetFieldType<T, P> = P extends `${infer Left}.${infer Right}`
  ? Left extends keyof T
    ? FieldWithPossiblyUndefined<T[Left], Right>
    : Left extends `${infer FieldKey}[${infer IndexKey}]`
      ? FieldKey extends keyof T
        ? FieldWithPossiblyUndefined<IndexedFieldWithPossiblyUndefined<T[FieldKey], IndexKey>, Right>
        : undefined
      : undefined
  : P extends keyof T
    ? T[P]
    : P extends `${infer FieldKey}[${infer IndexKey}]`
      ? FieldKey extends keyof T
        ? IndexedFieldWithPossiblyUndefined<T[FieldKey], IndexKey>
        : undefined
      : undefined

To retrieve a value from a tuple or array, there's a new GetIndexedField utility type. It returns tuple value by a given key, undefined if the key is out of tuple range, or element type for regular array. '0' extends keyof T condition checks if a value is a tuple, as arrays don't have string keys. If you know a better way to distinguish a tuple and an array, please let me know.

We are using ${infer FieldKey}[${infer IndexKey}] template to parse field[0] parts. Then, using the same Exclude | Extract technique as before, we are retrieving value types respecting optional properties.

Now we need to slightly modify our getValue function. For the sake of simplicity, I will replace .split('.') with .split(/[.[\]]/).filter(Boolean) to support new notation. That's probably not an ideal solution, but more complex parsing is out of the scope of the article.

Here's the final implementation



export function getValue<

  TData,

  TPath extends string,

  TDefault = GetFieldType<TData, TPath>

>(

  data: TData,

  path: TPath,

  defaultValue?: TDefault

): GetFieldType<TData, TPath> | TDefault {

  const value = path

    .split(/[.[]]/)

    .filter(Boolean)

    .reduce<GetFieldType<TData, TPath>>(

      (value, key) => (value as any)?.[key],

      data as any

    );

return value !== undefined ? value : (defaultValue as TDefault);

}

Conclusion

Now we not only have a nice utility function that improves code type safety, but also a better understanding of how to apply template literal and conditional types in practice.

I hope the article was helpful. Thank you for reading.

All code is available at this codesandbox

DEV Community: Aleksei Tsikov

Testing-library: avoid these mistakes in async tests

Table of Contents

Await with sync methods

Simple asynchronous request

Complex asyncronous request

Explanation

Conclusion

Async methods without await

Positive case

False-positive case

Explanation

Conclusion

Side-effects inside waitFor

How waitFor works

Green test

Hanging test

Explanation

Conclusion

How to avoid these issues

Summary

Advanced TypeScript: reinventing lodash.get

What is wrong with this approach?

Adding types

Parsing dot-separated paths

Adding a recursion

Handling optional properties

Implementing a utility function

Making component type-safe

Adding support for [] notation

Conclusion