Refactor integration test browser #935
Conversation
This commit prepares the testBrowser BrowserType init before the next refactoring commit. No behavior change is involved!
It is only used in a single place. There is no need to make it reusable at the moment.
This will be needed when options need to set testBrowser themselves. Instead of doing all the work in newTestBrowser and complicating it.
This will let us the testBrowser's initialization stage. Since it first makes a test VU and then uses it to initialize a browser type, some options (i.e. withLogCache) can only be used after the initialization. However, some others (i.e. withSamples) can only be used before the initialization. This fields lets us use both kind of options and let them init by detecting the init stage.
The option is now responsible for setting the log cache. This has the benefit of reducing the pollution in the newTestBrowser function and separates the responsibilities. We also don't need to keep track of whether to set the log cache. The option does that.
We're starting to remove the testBrowserOptions.
Simplify and remove testBrowserOptions. Options are now a part of testBrowser itself.
This makes it easier to see the initialization of the test browser as they're related.
|
@ankur22, thanks for your review 🙇
Sorry for the hassle 😢 I understand the challenge with the commit count! While I've added descriptive links to help, they may not be enough 😢 Despite this PR being smaller than usual, I acknowledge that breaking larger refactors into smaller atomic PRs could aid comprehension. We also need to ensure clarity is maintained due to too many PRs. How about discussing the best approach in a brief meeting? 🙂 I'm sure we can devise a suitable solution for all. Thanks for your input! 👍 |
Thanks, @ka3de 🙇 You mean the number of commits since our usual PRs are larger? Then again, sorry for the hassle :( I couldn't keep it smaller to make it a meaningful unit of work. I'm happy to discuss this in a short meeting :)
Good to hear! |
Updates: #898.
This PR improves the test browser into this one as follows. It's a nice first small step in removing the
apipackage, and starting with the core of tests seemed to me an effective idea. While on it, I also made a refactoring that had been on my mind for quite a while.apipackage dependency by using concrete types. See it here altogether.testBrowserOptionsortestBrowser.