add possibility to configure interval with configure method

[maciekstosio]

Summary

Currently, we're able to override the default timeout, but we can't do the same for the interval. For my test cases, 50ms has proven to be too long of an interval, and changing that setting for each case is tedious. I added a possibility to configure the default interval by configure method to solve that.

Test plan

I added tests based on those for interval and timeout.

[maciekstosio]

This seemed like a mistake based on what is in code (?)

[pierrezimmermannbam]

Indeed it is, thanks for fixing that!

[pierrezimmermannbam]

Thanks for your contribution @maciekstosio! This looks good to me, you'll need another maintainer's approval before this can be merged.

I'm curious though, why do you think that the 50 ms interval is too long for you?

[maciekstosio]

@pierrezimmermannbam I'm testing two screens, first one checks if the user is logged in, and it moves to another one that fetches data (and displays loader while fetch is in progress).

        render(
            <App
                tabsInitialRouteName='collection'
            />
        )
        
        await waitForElementToBeRemoved(() => screen.getByText('general.loading'))        
        await waitForElementToBeRemoved(() => screen.getByTestId('loading-indicator'))
        ...

Now, I had a problem that test failed on second waitForElementToBeRemoved when I was running multiple test cases, but succeeded when running alone. Fetch requests are mocked with nock. So I come up with an idea that mocked request can be just faster than 50ms and changing to 10ms helped. I assume it succeeded when running alone just because of some additional work on startup, but If you have some other ideas, I'm happy to discuss :)

[mdjastrzebski]

@maciekstosio regarding your test case, you should wait only for the first element to be removed, as waitForElementToBeRemoved requires element to be initially present and subsequently removed. For your second control you should just use:

expect(screen.getByTestId('loading-indicator')).not.toBeOnTheScreen()

[mdjastrzebski]

@maciekstosio I am somewhat hesitant on adding this option, as it seems that it's not really related to the root issue you are having. Relying on 10ms grain timing using real timers in test will probably result in flaky tests. The recommended solution would in such case would be to use fake timers, as they over more precise control.

Linking to similar DOM Testing Library issue.

[pierrezimmermannbam]

Oh ok I get it. I would agree with @mdjastrzebski that diminishing the interval for this use case may not be the optimal solution. Maybe for this particular use case, if you really want to test both loading states you could use the delay option from nock to ensure your API call doesn't resolve too early?

[maciekstosio]

Yeah, you're right. I had a problem using fake timers, thus I didn't like that approach, but it seems like it was because of nock don't work with it properly. I changed to fetch-mock-jest and it works as expected. Thanks for your suggestions

[pierrezimmermannbam]

Alright I'll close that pr for now then