feat: migrate useFetch to react-query #48

MohdImran001 · 2022-01-27T03:43:41Z

@aquibbaig PTAL. Some tests are failing with the error - e.g. Unable to find an element by: [data-testid="step-alert"] etc. On running in the browser, these data tags are present and the app is working perfectly fine but they are not rendered in tests.

aquibbaig · 2022-01-29T14:52:39Z

let me know when it is ready for a review

MohdImran001 · 2022-02-02T13:39:07Z

@aquibbaig I need some help regarding tests. After implementing react-query, five tests are failing.

  ● tests for the MachineSelector Component › is disabled and displays message on error

    Unable to find an element by: [data-testid="machine-selector"]

    <body
      class="chakra-ui-light"
    >
      <div
        id="chakra-toast-portal"
      >
        <ul
          id="chakra-toast-manager-top"
          style="position: fixed; z-index: 5500; pointer-events: none; display: flex; flex-direction: column; margin: 0px auto; top: 0px; right: 0px; left: 0px;"
        />
        <ul
          id="chakra-toast-manager-top-left"
          style="position: fixed; z-index: 5500; pointer-events: none; display: flex; flex-direction: column; top: 0px; left: 0px;"
        />
        <ul
          id="chakra-toast-manager-top-right"
          style="position: fixed; z-index: 5500; pointer-events: none; display: flex; flex-direction: column; top: 0px; right: 0px;"
        />
        <ul
          id="chakra-toast-manager-bottom-left"
          style="position: fixed; z-index: 5500; pointer-events: none; display: flex; flex-direction: column; bottom: 0px; left: 0px;"
        />
        <ul
          id="chakra-toast-manager-bottom"
          style="position: fixed; z-index: 5500; pointer-events: none; display: flex; flex-direction: column; margin: 0px auto; bottom: 0px; right: 0px; left: 0px;"
        />
        <ul
          id="chakra-toast-manager-bottom-right"
          style="position: fixed; z-index: 5500; pointer-events: none; display: flex; flex-direction: column; bottom: 0px; right: 0px;"
        />
      </div>
      <div>
        <div
          class="chakra-stack css-mgows9"
        >
          <div
            aria-valuemax="100"
            aria-valuemin="0"
            class="chakra-progress css-1hqez51"
            data-indeterminate=""
            role="progressbar"
          >
            <svg
              class="css-i5aqnf"
              viewBox="0 0 100 100"
            >
              <circle
                class="chakra-progress__track css-ol3i12"
                cx="50"
                cy="50"
                r="42"
                stroke-width="10px"
              />
              <circle
                class="chakra-progress__indicator css-zfuom8"
                cx="50"
                cy="50"
                r="42"
                stroke-width="10px"
              />
            </svg>
          </div>
        </div>
      </div>
    </body>

      56 |
      57 |     const { getByTestId } = render(<MachineSelector />);
    > 58 |     const selectElement = await waitFor(() => getByTestId("machine-selector"));
         |                                 ^
      59 |
      60 |     expect(selectElement).toBeDisabled();
      61 |     expect(getByTestId("error-message")).toBeTruthy();

      at waitForWrapper (node_modules/@testing-library/dom/dist/wait-for.js:173:27)
      at Object.<anonymous> (src/components/MachineSelector/machineSelector.test.tsx:58:33)

 FAIL  src/components/RouteSelector/routeSelector.test.tsx
  ● tests for the RouteSelector Component › is disabled and displays message on error

    Unable to find an element by: [data-testid="route-search"]

    <body
      class="chakra-ui-light"
    >
      <div
        id="chakra-toast-portal"
      >
        <ul
          id="chakra-toast-manager-top"
          style="position: fixed; z-index: 5500; pointer-events: none; display: flex; flex-direction: column; margin: 0px auto; top: 0px; right: 0px; left: 0px;"
        />
        <ul
          id="chakra-toast-manager-top-left"
          style="position: fixed; z-index: 5500; pointer-events: none; display: flex; flex-direction: column; top: 0px; left: 0px;"
        />
        <ul
          id="chakra-toast-manager-top-right"
          style="position: fixed; z-index: 5500; pointer-events: none; display: flex; flex-direction: column; top: 0px; right: 0px;"
        />
        <ul
          id="chakra-toast-manager-bottom-left"
          style="position: fixed; z-index: 5500; pointer-events: none; display: flex; flex-direction: column; bottom: 0px; left: 0px;"
        />
        <ul
          id="chakra-toast-manager-bottom"
          style="position: fixed; z-index: 5500; pointer-events: none; display: flex; flex-direction: column; margin: 0px auto; bottom: 0px; right: 0px; left: 0px;"
        />
        <ul
          id="chakra-toast-manager-bottom-right"
          style="position: fixed; z-index: 5500; pointer-events: none; display: flex; flex-direction: column; bottom: 0px; right: 0px;"
        />
      </div>
      <div>
        <div
          class="chakra-stack css-mgows9"
        >
          <div
            aria-valuemax="100"
            aria-valuemin="0"
            class="chakra-progress css-1hqez51"
            data-indeterminate=""
            role="progressbar"
          >
            <svg
              class="css-i5aqnf"
              viewBox="0 0 100 100"
            >
              <circle
                class="chakra-progress__track css-ol3i12"
                cx="50"
                cy="50"
                r="42"
                stroke-width="10px"
              />
              <circle
                class="chakra-progress__indicator css-zfuom8"
                cx="50"
                cy="50"
                r="42"
                stroke-width="10px"
              />
            </svg>
          </div>
        </div>
      </div>
    </body>

      64 |
      65 |     const { getByTestId } = render(<RouteSelector />);
    > 66 |     const searchElement = await waitFor(() => getByTestId("route-search"));
         |                                 ^
      67 |
      68 |     expect(searchElement).toBeDisabled();
      69 |     expect(getByTestId("error-message")).toBeTruthy();

      at waitForWrapper (node_modules/@testing-library/dom/dist/wait-for.js:173:27)
      at Object.<anonymous> (src/components/RouteSelector/routeSelector.test.tsx:66:33)

 FAIL  src/components/GraphWrapper/graphWrapper.test.tsx (5.335 s)
  ● tests for the Graph Component › shows error dialog if route is selected and error is thrown

    Unable to find an element by: [data-testid="graph-error"]

    <body
      class="chakra-ui-light"
    >
      <div
        id="chakra-toast-portal"
      >
        <ul
          id="chakra-toast-manager-top"
          style="position: fixed; z-index: 5500; pointer-events: none; display: flex; flex-direction: column; margin: 0px auto; top: 0px; right: 0px; left: 0px;"
        />
        <ul
          id="chakra-toast-manager-top-left"
          style="position: fixed; z-index: 5500; pointer-events: none; display: flex; flex-direction: column; top: 0px; left: 0px;"
        />
        <ul
          id="chakra-toast-manager-top-right"
          style="position: fixed; z-index: 5500; pointer-events: none; display: flex; flex-direction: column; top: 0px; right: 0px;"
        />
        <ul
          id="chakra-toast-manager-bottom-left"
          style="position: fixed; z-index: 5500; pointer-events: none; display: flex; flex-direction: column; bottom: 0px; left: 0px;"
        />
        <ul
          id="chakra-toast-manager-bottom"
          style="position: fixed; z-index: 5500; pointer-events: none; display: flex; flex-direction: column; margin: 0px auto; bottom: 0px; right: 0px; left: 0px;"
        />
        <ul
          id="chakra-toast-manager-bottom-right"
          style="position: fixed; z-index: 5500; pointer-events: none; display: flex; flex-direction: column; bottom: 0px; right: 0px;"
        />
      </div>
      <div>
        <div
          class="chakra-stack css-mgows9"
        >
          <div
            aria-valuemax="100"
            aria-valuemin="0"
            class="chakra-progress css-vj5gvp"
            data-indeterminate=""
            role="progressbar"
          >
            <svg
              class="css-1g822n4"
              viewBox="0 0 100 100"
            >
              <circle
                class="chakra-progress__track css-ol3i12"
                cx="50"
                cy="50"
                r="42"
                stroke-width="10px"
              />
              <circle
                class="chakra-progress__indicator css-zfuom8"
                cx="50"
                cy="50"
                r="42"
                stroke-width="10px"
              />
            </svg>
          </div>
        </div>
      </div>
    </body>

      51 |       </>
      52 |     );
    > 53 |     const graphError = await waitFor(() => getByTestId("graph-error"));
         |                              ^
      54 |
      55 |     expect(graphError).toBeTruthy();
      56 |     expect(mockedAxios.get).toHaveBeenCalledWith(

      at waitForWrapper (node_modules/@testing-library/dom/dist/wait-for.js:173:27)
      at Object.<anonymous> (src/components/GraphWrapper/graphWrapper.test.tsx:53:30)

  ● tests for the Graph Component › shows error to increase step value if data more than 6000

    Unable to find an element by: [data-testid="step-alert"]

    <body
      class="chakra-ui-light"
    >
      <div
        id="chakra-toast-portal"
      >
        <ul
          id="chakra-toast-manager-top"
          style="position: fixed; z-index: 5500; pointer-events: none; display: flex; flex-direction: column; margin: 0px auto; top: 0px; right: 0px; left: 0px;"
        />
        <ul
          id="chakra-toast-manager-top-left"
          style="position: fixed; z-index: 5500; pointer-events: none; display: flex; flex-direction: column; top: 0px; left: 0px;"
        />
        <ul
          id="chakra-toast-manager-top-right"
          style="position: fixed; z-index: 5500; pointer-events: none; display: flex; flex-direction: column; top: 0px; right: 0px;"
        />
        <ul
          id="chakra-toast-manager-bottom-left"
          style="position: fixed; z-index: 5500; pointer-events: none; display: flex; flex-direction: column; bottom: 0px; left: 0px;"
        />
        <ul
          id="chakra-toast-manager-bottom"
          style="position: fixed; z-index: 5500; pointer-events: none; display: flex; flex-direction: column; margin: 0px auto; bottom: 0px; right: 0px; left: 0px;"
        />
        <ul
          id="chakra-toast-manager-bottom-right"
          style="position: fixed; z-index: 5500; pointer-events: none; display: flex; flex-direction: column; bottom: 0px; right: 0px;"
        />
      </div>
      <div>
        <div
          class="chakra-stack css-mgows9"
        >
          <div
            aria-valuemax="100"
            aria-valuemin="0"
            class="chakra-progress css-vj5gvp"
            data-indeterminate=""
            role="progressbar"
          >
            <svg
              class="css-1g822n4"
              viewBox="0 0 100 100"
            >
              <circle
                class="chakra-progress__track css-ol3i12"
                cx="50"
                cy="50"
                r="42"
                stroke-width="10px"
              />
              <circle
                class="chakra-progress__indicator css-zfuom8"
                cx="50"
                cy="50"
                r="42"
                stroke-width="10px"
              />
            </svg>
          </div>
        </div>
      </div>
    </body>

      73 |       </>
      74 |     );
    > 75 |     const stepAlert = await waitFor(() => getByTestId("step-alert"));
         |                             ^
      76 |
      77 |     expect(stepAlert).toBeTruthy();
      78 |     expect(mockedAxios.get).toHaveBeenCalledWith(

      at waitForWrapper (node_modules/@testing-library/dom/dist/wait-for.js:173:27)
      at Object.<anonymous> (src/components/GraphWrapper/graphWrapper.test.tsx:75:29)

  ● tests for the Graph Component › plots the data correctly

    expect(jest.fn()).toHaveBeenCalledWith(...expected)

    Expected: "http://localhost:9990/api/v1/query-entity?name=/srv/modular_savings_health.skt.t3&start=2022-02-02T12:33:45.465Z&end=2022-02-02T13:33:45.465Z&step=15"

    Number of calls: 0

       98 |
       99 |     expect(stepAlert).toBeFalsy();
    > 100 |     expect(mockedAxios.get).toHaveBeenCalledWith(
          |                             ^
      101 |       queryEntities(
      102 |         mockSelectedRoutePath,
      103 |         defaultStartTimestamp,

      at Object.<anonymous> (src/components/GraphWrapper/graphWrapper.test.tsx:100:29)

Test Suites: 3 failed, 2 passed, 5 total
Tests:       5 failed, 12 passed, 17 total
Snapshots:   0 total
Time:        6.238 s
Ran all test suites.

Watch Usage: Press w to show more.

MohdImran001 · 2022-02-02T13:50:54Z

@aquibbaig, in the machineSelector.test.tsx. we have the following failed test.

  test("is disabled and displays message on error", async () => {
    mockedAxios.get.mockRejectedValue(new Error("Async error"));

    const { getByTestId } = render(<MachineSelector />);
    const selectElement = await waitFor(() => getByTestId("machine-selector"));

    expect(selectElement).toBeDisabled();
    expect(getByTestId("error-message")).toBeTruthy();
  });

I am using isError flag returned by react-query when there is an error. In machineSelector component, it is used as follows
so when there is an error, the component only returns this part.

  if (isError) {
    return (
      <Alert data-testid="error-message" fontSize="xs" status="error">
        <AlertIcon />
        {error?.response?.data}
      </Alert>
    );
  }

because we are looking for [data-testid="machine-selector"], it doesn't exists in the above snippet, therefore we get the following error.
Unable to find an element by: [data-testid="machine-selector"]

Other failed tests are also having similar problem. I am new to testing, can you please look at this and suggest some approach. 😄

ruddi10 · 2022-02-02T14:13:37Z

Hey @MohdImran001 actually I am seeing that in a PR where we made the flow of our code consistent there we made some changes in the way the things will appear on error. However the same was not updated in the unit tests. For eg- earlier in machineselector component we used to show the error as well as the disabled dropdown but now we are just showing the error and no dropdown hence the test is failing as old test are expecting the dropdown too

MohdImran001 · 2022-02-02T15:20:15Z

@ruddi10 if this PR looks good to you, I guess we can merge it. We can refactor the tests in a different PR.

MohdImran001 · 2022-02-02T15:22:16Z

After that, we can also set up a CI for running tests.

aquibbaig · 2022-02-05T16:05:09Z

@MohdImran001 I think you have the wrong notion about tests, the tests are there in the first place so that we don't get errors later.

aquibbaig · 2022-02-05T16:08:12Z

The migration from react-query is not as simple, we need to have a dedicated hooks/ folder(either localised or global) which contains various react-query hooks for each action. For example: For fetching the routes, we can have a hook useRoutes.ts with the corresponding implementation (the cache key and what causes it to refetch). Also, we need to get rid of useFetch completely

MohdImran001 · 2022-02-05T19:06:39Z

Sure, I will create different hooks for each action. I would like to know what can be done to fix the failing tests. We might need to rewrite them to suit the current component structure. I would love to know your thoughts on this. @aquibbaig

aquibbaig · 2022-02-05T21:01:56Z

I think this will be a very big task and commitment, why don't we break it down into smaller bits and pieces.

Install React Query (package)
Configuration (initialize query client, providers, etc)
Add page query hooks
Remove useFetch from the codebase

aquibbaig · 2022-02-05T21:05:45Z

Btw, we need to investigate why the tests are failing here, this is failing in master:branch too.

MohdImran001 · 2022-02-06T06:12:31Z

Hey @MohdImran001 actually I am seeing that in a PR where we made the flow of our code consistent there we made some changes in the way the things will appear on error. However the same was not updated in the unit tests. For eg- earlier in machineselector component we used to show the error as well as the disabled dropdown but now we are just showing the error and no dropdown hence the test is failing as old test are expecting the dropdown too

The test are failing because of the reason mentioned above. @aquibbaig

MohdImran001 added 2 commits January 26, 2022 17:36

fix: prettier errors

b6f152b

feat: migrate useFetch to react-query

7f2ad67

MohdImran001 marked this pull request as ready for review February 2, 2022 15:22

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: migrate useFetch to react-query #48

feat: migrate useFetch to react-query #48

MohdImran001 commented Jan 27, 2022 •

edited

Loading

aquibbaig commented Jan 29, 2022

MohdImran001 commented Feb 2, 2022

MohdImran001 commented Feb 2, 2022

ruddi10 commented Feb 2, 2022

MohdImran001 commented Feb 2, 2022

MohdImran001 commented Feb 2, 2022

aquibbaig commented Feb 5, 2022

aquibbaig commented Feb 5, 2022

MohdImran001 commented Feb 5, 2022

aquibbaig commented Feb 5, 2022

aquibbaig commented Feb 5, 2022 •

edited

Loading

MohdImran001 commented Feb 6, 2022

feat: migrate useFetch to react-query #48

Are you sure you want to change the base?

feat: migrate useFetch to react-query #48

Conversation

MohdImran001 commented Jan 27, 2022 • edited Loading

aquibbaig commented Jan 29, 2022

MohdImran001 commented Feb 2, 2022

MohdImran001 commented Feb 2, 2022

ruddi10 commented Feb 2, 2022

MohdImran001 commented Feb 2, 2022

MohdImran001 commented Feb 2, 2022

aquibbaig commented Feb 5, 2022

aquibbaig commented Feb 5, 2022

MohdImran001 commented Feb 5, 2022

aquibbaig commented Feb 5, 2022

aquibbaig commented Feb 5, 2022 • edited Loading

MohdImran001 commented Feb 6, 2022

MohdImran001 commented Jan 27, 2022 •

edited

Loading

aquibbaig commented Feb 5, 2022 •

edited

Loading