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

feat: spies #162

Merged
merged 32 commits into from
May 26, 2022
Merged

feat: spies #162

merged 32 commits into from
May 26, 2022

Conversation

crookse
Copy link
Member

@crookse crookse commented Apr 20, 2022

Closes #151

TODO

  • Write tests for VerificationError
  • Write tests for spies on classes
  • Write tests for spies on object methods
  • Write tests for spies on function expressions
  • Write tests for spies on fakes
  • Write tests for spies in tests/cjs and tests/esm
  • Write tests for FunctionExpressionVerifier
  • Write tests for MethodVerifier
  • Write documentation in drashland/webiste-v2 repo (feat(rhum/v2.x): spies website-v2#158)

Summary

  • feat: Adds spies
    • Users can create a spy out of a class via const spyClass = Spy(SomeClass)
    • Users can spy on an object's method via const spyMethod = Spy(obj, "nameOfMethod", optionalReturnValue)
    • Users can spy on a function expression via const spyMethod = Spy(func, optionalReturnValue)
    • Limitations:
      • When using CommonJS, there is no way for Rhum to differentiate between classes and function expressions. This means spying on function expressions in a CommonJS codebase is not supported.
  • feat: Add FunctionExpressionVerifier for Spy stub call verification
  • refactor: Add CallableVerifier that MethodVerifier can extend
    • This was done to allow shared verification logic between the new FunctionExpressionVerifier for Spy stubs and the MethodVerifier used between the Mock and the Spy test doubles
  • refactor: Alphabetize methods, properties, error classes, interfaces, and types
  • refactor: Improved VerificationError class (formerly MethodVerificationError)
    • Changed the name because it's a generic verification error class, not just a method verification. It's also used for function expression verifications.
    • Adds more lines to the ignoredLines array so that we don't accidentally show a random file.
    • Fixes issue with the stack trace sometimes showing the problematic file twice. If it's shown twice, then we just slice off the duplicate.
  • refactor: clean up a lot of the code
    • DRY code more
    • There are three verifiers (one for mocks, one for spy methods, and one for spy function expressions). These duplicated a lot of code. They now extend a parent class (CallableVerifier) to reduce the amount of duplicated code.
    • Remove dead code (e.g., MethodArguments type)
    • Change semantics of variables and method names to better reflect what they are doing and what they are used for. For example, MethodCalls is now MockMethodCalls because it only pertains to mocks.
  • docs: Add all necessary doc blocks to internal code and interfaces
    • Improves type-hinting for the user (you can see how it's working by looking using the tests in visual studio code)
    • Improves documentation in general (for our development efforts)
    • Adds hella lines of code to this pull request
    • Consolidates doc block descriptions to reference interfaces. Not sure why I was like, "Let's document this here, then document it again here." So, I just referenced the interface like "See IMock.calls".
  • build: Update build process for CJS and ESM
    • Adds spy-related files
    • Adds verifiers
    • Fixes issue with stacked import/export statements. For example, the following wasn't working and now it does:
      import {
       something,
       something,
      something
      } from "some/path/file.ts"
      
      export {
       something,
       something,
      something
      } from "some/path/file.ts"

Copy link
Member

@ebebbington ebebbington left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

one comment, but no biggie, overral LGTM :D Some really nice code here, of course didnt check jest generated, and im happy if you think you've hit enough during the tesst

@@ -99,6 +99,7 @@ export class MockBuilder<ClassToMock> extends TestDoubleBuilder<ClassToMock> {
value: (...args: unknown[]) => {
// Track that this method was called
mock.calls[method]++;
// TODO: copy spy approach because we need mock.expected_args
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

missed todo? not sure if this is crucial or could be done later down the line

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i didn't want to bloat the PR more with hella changes so i added these two issues:

#166
#167

@ebebbington
Copy link
Member

image

@crookse crookse added the Type: Minor Merging this pull request results in a minor version increment label May 1, 2022
@crookse crookse requested a review from Guergeiro May 8, 2022 15:16
@crookse crookse marked this pull request as ready for review May 8, 2022 15:20
@crookse crookse merged commit 2006af5 into chore/dry-code May 26, 2022
@crookse crookse mentioned this pull request May 30, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Type: Minor Merging this pull request results in a minor version increment
Development

Successfully merging this pull request may close these issues.

feat(v2): spies
3 participants