callstack / react-native-testing-library

🦉 Simple and complete React Native testing utilities that encourage good testing practices.
https://callstack.github.io/react-native-testing-library/
MIT License
3.07k stars 271 forks source link

RFC: impact of `detectHostComponentNames` #1579

Open mdjastrzebski opened 6 months ago

mdjastrzebski commented 6 months ago

Summary

I've been curious what is the impact of automatically detecting host component names. Due to the lack of better large test suite I've taken RNTL own tests.

Here is my process:

  1. Disable tests that fail with hardcode hostComponentNames with test.skip().
  2. Measure execution time with time yarn test in relevant scenarios
  3. Add console.log() to detectHostComponentNames() and count execution count (separate run from measuring time).

Note: we also have beforeEach(() => resetToDefaults()), so I measured it with and without it.

Seems like Jet by default clears the runtime state per each file. So we have detectHostComponentNames() being called for each file.

Here are my questions:

thymikee commented 6 months ago

It's expected from Jest to recreate the whole environment for each test suite (file).

pierrezimmermannbam commented 6 months ago

You're running the whole RNTL test suite? On my machine it takes about 7s, does it take only 39,4ms on yours? From the tests I'v run on my machine (M3 pro) the difference is less significant, it takes on average 7.45s to run the test suite without the detect and 7.7s

Other than that yeah the approach is correct. One thing to note though is that I would expect the difference to be even less significant on a real project where tests per file are likely to be more heavy (I'll try to test it on my current project too). I don't know if the perf impact is high enough to look into alternative approaches, I guess if we find a good one it could be worth

mdjastrzebski commented 5 months ago

@pierrezimmermannbam The unit was to be s not ms (sorry for confusion, I've run it on my MBP with Intel)

pierrezimmermannbam commented 5 months ago

Oh I see, that makes sense. Strange that the difference in percentage would be so different on our machines. Also I'm just realizing this shows that calling resetToDefaults is useless no? Why were we doing this initially?

mdjastrzebski commented 5 months ago

Re %, I've also figured out that is swapped the 13% and 20% (the absolute numbers where correct). Now it should be good in the issue desc.

Re resetToDefaults. I guess that the justification is to make sure that each test is indeed separated from the others (in the same file; as normally tests are separated between files). We could either keep it or remove it, my worry was rather about our users than us.