Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

The Big Epic Rewrite Issue #263

Open
7 of 31 tasks
searls opened this issue Jun 17, 2017 · 0 comments
Open
7 of 31 tasks

The Big Epic Rewrite Issue #263

searls opened this issue Jun 17, 2017 · 0 comments
Assignees

Comments

@searls
Copy link
Member

searls commented Jun 17, 2017

Starting with the setup work in #255, I've been undergoing an in-place rewrite of testdouble.js that essentially dogfoods both the library & Discovery Testing by replacing the library bit-by-bit.

This has been incredibly challenging for a variety of reasons, among them: keeping oneself unanchored by existing implementation when trying to think of a better design, coming up with useful names for (relative to most JS libs) very meta concepts, and resisting the urge to fix problems and incongruities in the existing implementation as I go. If this rewrite also substantially changes the public-facing behavior of the library, it's going to be like tuning two knobs on a radio at once, making everything more difficult.

Therefore, this thread will maintain 3 to-do lists for tracking progress of the rewrite. First: the actual progress so far incorporating the new implmentation. Second: the future work to undertake to "fix" what I identify as broken with the library after we call the rewrite "done". Third: internal improvements that should be tracked and made as part of the rewrite.

Progress so far.

testdouble.js has a pretty straightforward public-API:

  • test double creation
    • td.function (alias td.func)
    • td.object
    • td.constructor
  • td.replace
    • CommonJS module replacement (Node.js only)
    • named property replacement
  • td.when for stubbing
  • td.verify for verifying calls
  • matchers for both stubbing and verifying
    • the same built-in matchers we have today
    • the same matcher-creator function we provide today
    • the special matchers like captor and callback which enable other behavior
  • td.reset for resetting test double state and undoing dependency replacement
  • td.explain for providing metadata about a fake thing
  • td.config for global config options (log level, which promise constructor to use, etc)
  • Internal bits
    • A logging facility
    • An in-memory store for calls (CallLog)
    • An in-memory store for stubbings (StubbingRegister)

*being labeled complete here does not mean they are wired up in the deployed library, yet. They'll sit in the repo as inert code until they can go end-to-end. For instance, td.func() can't be replaced until its calls go in the same call log that td.verify() is looking at, otherwise the library wouldn't work. It truly is a big epic rewrite as a result, since almost everything depends on everything else because of how stateful a library this is.

Room to improve

Here's what I've identified as things to improve while working through everything. Some of these feel urgent enough that I'll incorporate them into the current implementation to avoid rewriting a bad thing.

  • Improve td.explain to accept any fake object/function/constructor and give you a way to navigate to its parents and children if it was a property of a fake thing
  • Get consistent about finding all properties on a function or object to be cloned or faked, that means enumerable/non-enumerable, inherited/owned. Currently all 3 creation methods identify these differently td.replace throws with objects created with Object.create #257
  • When replacing a function, replace it as a "plain" function or a constructor function consistently. Currently some places get the "if a constructor replace it this way" treatment and some don't. Getting consistent about how all functions are replaced will eliminate a lot of branching
  • Do "deep" cloning & property faking as opposed to single-layer, since we can no longer reasonably assert the opinion that "good" modules be one or two layers deep, thanks to ES modules having a necessary layer beyond CJS ones Single-layer property replacement is inappropriate for ES modules #262
  • Support ES getters and setters (right now, we're punting on getters & setters entirely; they dramatically complicate how we identify and track "properties" on objects)

Internal improvements

There are some things that can be improved/refactored without affecting public-facing behavior. Tracking those here:

  • td.func
    • instead of having magical fake functions flying about, create an actual Double value that knows a lot of metadata about the concept of a double (like the real thing its masking)
    • tracking parent/child relationships, improve the name of each test double function to concatenate all the names of its parents so td.explain and error messages make it clear which test double is which
    • today's td.function takes a second __optionalName argument due to a quirk in how node-module replacement was written. This can be fixed by making
  • td.replace
    • for some reason the name of test doubles on default-export ES modules is just .default which means they all look the same. the name should include the module path + the export name
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

1 participant