Optional loading state skipping for mocked requests

[guillermoparral1995]

Motivation

In our project we have setup visual tests of screens that relied on Apollo for fetching information before rendering. These requests were being mocked by setting a MockLink with mocked responses, but we noticed that visual tests were failing because the screen capture was being made in a transitional state between loading and loaded states, which for our use case was undesired.

This solution was inspired by reading about a similar situation in StackOverflow

Proposed solution

We noticed that the issue was inside MockLink's request function, where the actual returning of the mocked responses is wrapped inside a setTimeout function, which forces a loading state even when the mock has explicitly specified a delay of 0. In our project we have copied the MockLink implementation by copying the project and modifying the offending section with the following:

    const handleObserver = (
      observer: ZenObservable.SubscriptionObserver<
        FetchResult<
          {
            [key: string]: any
          },
          Record<string, any>,
          Record<string, any>
        >
      >
    ) => {
      if (configError) {
        try {
          // The onError function can return false to indicate that
          // configError need not be passed to observer.error. For
          // example, the default implementation of onError calls
          // observer.error(configError) and then returns false to
          // prevent this extra (harmless) observer.error call.
          if (this.onError(configError, observer) !== false) {
            throw configError
          }
        } catch (error) {
          observer.error(error)
        }
      } else if (response) {
        if (response.error) {
          observer.error(response.error)
        } else {
          if (response.result) {
            observer.next(
              typeof response.result === 'function'
                ? (response.result as ResultFunction<FetchResult>)()
                : response.result
            )
          }
          observer.complete()
        }
      }
    }

    return new Observable((observer) => {
      let timer: ReturnType<typeof setTimeout> | undefined
      // we keep the original implementation if delay is explicitly set
      if (response?.delay) {
        timer = setTimeout(() => {
          handleObserver(observer)
        }, response.delay)
      } else {
        // it no delay is set, then we directly return the mocked responses without setTimeout
        handleObserver(observer)
      }

      return () => {
        if (timer) clearTimeout(timer)
      }
    })

[jerelmiller]

Hey @guillermoparral1995 👋

This is an interesting idea. I'd challenge this request a bit to understand if this is specific to your use case or if you think this should be considered a broader feature.

Given that MockLink is primarily a testing utility, we want to do our best to provide best-practice utilities that resemble how a production app will make the request (granted a mock can only go so far toward this goal). We love Testing Libary's philosophy here of "The more your tests resemble the way your software is used, the more confidence they can give you" and believe we should adhere to that principle as much as we can.

While a delay of 0 should feel like there is no sort of delay (i.e. no setTimeout), a setTimeout of 0 still provides some level of asynchrony. Removing that setTimeout entirely means your component actually behaves completely differently because that request is done synchronously, skipping the loading state entirely.

It's extremely rare that you're going to be running GraphQL queries against a synchronous data source, so we feel that straying away from the asynchronous nature violates the philosophy stated above. This helps avoid tests that end up with false positives where a component works in your test, but breaks in production because it didn't handle the loading state correctly.

Given your situation, creating a custom link that makes the request synchronous makes a lot of sense! No need to use MockLink if it doesn't fit your use case very well. Let me know what you think and if my reasoning makes sense.

[guillermoparral1995]

Hi there @jerelmiller!

That reasoning makes a lot of sense, but something that was concerning for us was the fact that by creating a custom MockLink for our use case we would be missing updates to this implementation that may come in future versions, and considering that we're making a relatively small change we wouldn't want to miss on those.

Would it make sense to somehow enable this kind of behaviour through a specific config? Maybe even tagging it like "specific for visual tests" or something of the sort, since we're not testing actual GraphQL behaviour but rather just want to have specific content returned to us in tests.

[jerelmiller]

@guillermoparral1995 apologies I missed your reply!

something that was concerning for us was the fact that by creating a custom MockLink for our use case we would be missing updates to this implementation that may come in future versions

This is definitely a valid concern and can understand where you're coming from. I can't guarantee how much we will/won't change, but I can at least say that because these are testing utilities (testing meaning automated tests i.e. jest), we would likely make updates for the use-case of testing only. This might have further negative impact on what you're trying to accomplish with your use case.

Would it make sense to somehow enable this kind of behaviour through a specific config?

To reiterate from my previous statement, that loading state is crucial in your tests to more closely resemble what your component will see in production, otherwise you might end up with a false-positive in your test. I know from experience that as soon as you introduce a behavior into something, devs WILL use it, and it becomes very difficult to walk back. This is the reason I'm so hesitant to entertain this idea of removing the loading state in any capacity, because it introduces bad practices into your testing environment that people WILL use (regardless of the advice we give).

---

There are some additional behaviors of MockLink that may also be less desirable anyways. For example, most people don't realize that the mocks array that you pass into MockLink/MockedProvider are mutated during the course of the test. MockLink splices the matched mock out of that array, which means that if you need to "call" that mock more than once, you may end up with an empty array of mocks.

In a testing environment, this is usually ok because you have full control over the number of times something is fetched. This particular "feature" of the mocks makes it easier to simulate a refetch or subsequent fetch on the same query/variables, but return (potentially) different results.

For something that is meant to show static data, this behavior might become a nightmare, because you'll start getting errors in your app the moment you've emptied that mocks array.

I'd instead recommend creating something like a StaticLink that you maintain that avoids some of the non-optimal behavior in the mock link (i.e. the array splicing). It can certainly borrow some of the semantics from MockLink. I think you'd be much happier with this approach than trying to shoehorn in a testing utility that wasn't designed for this use-case.

Let me know what you think! I'd be happy to provide a base implementation of StaticLink in a GitHub gist if you're interested in what that might look like.

[guillermoparral1995]

@jerelmiller Hi there! Now I missed your reply 🤣

I think that what you say makes a lot of sense :) I'd love to see an example of StaticLink, give that a shot and share the results with you!