Skip to content

NoIssue - filterState tests improvements

This MR consists of several improvements on filterState tests.

These changes were not discussed in detail before, and there was no plan for it AFAIK, but because of recent prior work, it developed quickly, and I thought it might be worthy of merging. Even if it is not merged, I believe it is at least useful for raising discussion topics, and MR can be updated as more opinions join.

Changes:

  • Test structure is changed. It is IMO more developer friendly, code duplication decreased, reports are more consistent and readable.
  • All tests are rewritten. Overcovering tests are removed. LOC significantly decreased.
  • Added a small test toolkit, removing need for re-declaring many variables and functions. (*)

(*):

  • Our test setup is missing tools, namely eventListeners, spies and stubs.
  • assert we are using is limited for these uses. (nothing for events, and callTracker is not developer friendly)
  • Ideally, I would use a proper library for these uses, but because we didn't discuss this before, I didn't want to add another dependency.
  • Since we already have chai dependency, I wanted to use it. But unfortunately it lacks these tools too. While it has some plugin support, plugins are very outdated. (ex: only 2 event plugins are last updated in 2013 and 2016)
  • I added listen for capturing events, spy for tracking function calls, collect for accumulating generators and stub for replacing functions.
  • After some discussion and planning, we should still try to use a proper library IMHO, but for now this should be sufficient and serves its purpose.
  • After discussion, I would like to move these test tools to its own module, and possibly use in other tests. (Patterns they override are very common in our code base, mostly duplicated.)
Edited by Ozan Ozbek

Merge request reports