Skip to content
Snippets Groups Projects

Implement custom rop rspec matcher

Merged
Safwan Ahmedrequested to merge
implement-custom-rop-rspec-matcher into master

What does this MR do and why?

Issue: Implement custom Rspec matcher for ROP main cla... (#455126 - closed)

This change aims to improve testing experience by reducing the amount of code needed to write tests for ROP main classes. It does this by implementing a custom rspec matcher for ROP chain classes. This change also incorporates ideas from Allow arity -1 in ROP to improve testability (!151574 - closed) which allows us to directly mock functions and Use meta programming to make testing remote dev... (!152657 - closed) to determine what methods need to be mocked (railway classes always have single public method)

How the matcher works

The matcher works by:

  • listing railway classes that are expected to be called and their expected result passing method (and_then or map)
  • stating the initial value passed
  • stating the expected return

like so:

 expect_railway_to_have_been_called([settings_initializer_class, :map],
        [current_settings_reader_class, :and_then],
        [extensions_gallery_metadata_generator_class, :map],
        [env_var_reader_class, :and_then]).with(input_value).and_return(
          Result.err(RemoteDevelopment::Messages::SettingsEnvironmentVariableReadFailed.new(err_message_context))
        )

Important notes:

  • We need to determine the passer method as when propagating the input through the chain we need to know if the mocked method should return a plain value or a Result type
  • the matcher uses the initial input, clones it and makes that clone be the expected input by the next rop class in the chain
  • the last class in the chain is setup to return the value passed to and_return

Compromises and issues

To keep the code needed for the matcher minimum, I avoided passing information about what class might return an error in the tuple e.g: [settings_initializer_class, :map, error]. This presented an issue:

When the test is expected to return an Result.err but ends with a map, the stubbing of the last chain class method wont work as map cannot return an error. To remedy this, you can limit the classes whose methods you want to mock in the rspec matcher to the first class in the chain that can return an error. I think this is acceptable because even if you are to mock methods down the chain after an earlier error occurs, you can not verify if the methods were called as errors are simply forwarded by the result class.

MR acceptance checklist

Please evaluate this MR against the MR acceptance checklist. It helps you analyze changes to reduce risks in quality, performance, reliability, security, and maintainability.

Screenshots or screen recordings

Screenshots are required for UI changes, and strongly recommended for all other merge requests.

Before After

How to set up and validate locally

Numbered steps to set up and validate the change are strongly suggested.

Edited by Safwan Ahmed

Merge request reports

Loading
Loading

Activity

Filter activity
  • Approvals
  • Assignees & reviewers
  • Comments (from bots)
  • Comments (from users)
  • Commits & branches
  • Edits
  • Labels
  • Lock status
  • Mentions
  • Merge request status
  • Tracking
Please register or sign in to reply