Skip to content
This repository was archived by the owner on Aug 18, 2020. It is now read-only.

some cleanups in preparation for adding new functionalities #3743

Closed
wants to merge 1 commit into from

Conversation

MarcFontaine
Copy link
Contributor

@MarcFontaine MarcFontaine commented Oct 12, 2018

Description

Most of this PR consists of refactorings that don't change functionality.
The motivations for this PR are to clean up parts of the code and to make it easier to add new features.

Changes:

  • Main.hs: has been reduced to a minimum to make it easy to read and modifiy.
    Functions that are related to the RandomStateWalk have been moved to that module.
    Functions for setting up the WalletClient have been moved to SetupTestEnv.hs.
  • SetupTestEnv.hs
    exports exports setupClient :: CLIOptions -> IO (WalletClient IO, Manager)
    which has been inlined in main :: IO () before.
    setupClient is needed to run tests and single queries from the REPL.
  • Functions.hs / RandomStateWalk.hs
    The module Functions.hs has been renamed to RandomStateWalk.hs.
    some weeks ago some 5 lines have been added to Functions.hs that do not deal with the random state walk. these have been moved to Utils.hs
    With this change the module RandomStateWalk.hs contains all random-state-walk code and nothing else.
    The random-state-walk has been disabled some weeks ago is dead code (is compiled but does not run)

There is also one breaking change in the way arguments are forwarded to HSpec.

The wallet integration tests now uses --hspec-options to pass options to HSpec.
For example:

$ nix-build -A walletIntegrationTests -o launch_integration_tests
$ ./launch_integration_tests --hspec-options '--match Address --seed 47286650'
Motivation :

The old version defined local options --seed and --match which are then repacked and passed to HSpec.
Problems with the old approach:

  • It does not make clear that --seed and --match are indeed HSpec options. (its not clear where to lookup the documentation etc)
  • It does not scale and does not support all the other
    useful options of HSpec
  • The implementation uses an anti-pattern. That pattern uses
    2 lines for one option, 4 lines for two option, 8 lines for three options...
    it makes it difficult to add new options.

Linked issue

CO-424

Type of change

  • 🐞 Bug fix (non-breaking change which fixes an issue)
  • 🛠 New feature (non-breaking change which adds functionality)
  • ⚠️ Breaking change (fix or feature that would cause existing functionality to change)
  • 🏭 Refactoring that does not change existing functionality but does improve things like code readability, structure etc
  • 🔨 New or improved tests for existing code
  • ⛑ git-flow chore (backport, hotfix, etc)

Developer checklist

  • I have read the style guide document, and my code follows the code style of this project.
  • If my code deals with exceptions, it follows the guidelines.
  • I have updated any documentation accordingly, if needed. Documentation changes can be reflected in opening a PR on cardanodocs.com, amending the inline Haddock comments, any relevant README file or one of the document listed in the docs directory.
  • CHANGELOG entry has been added and is linked to the correct PR on GitHub.

Testing checklist

  • I have added tests to cover my changes.
  • All new and existing tests passed.

QA Steps

Screenshots (if available)

@KtorZ
Copy link
Contributor

KtorZ commented Oct 15, 2018

Closing in favor of #3751

@KtorZ KtorZ closed this Oct 15, 2018
@MarcFontaine MarcFontaine deleted the mafo/CO-242-CHW-tests branch March 19, 2020 11:30
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants