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

CO-424 some cleanups in preparation for adding new functionalities #3751

Merged
merged 1 commit into from
Oct 17, 2018

Conversation

MarcFontaine
Copy link
Contributor

@MarcFontaine MarcFontaine commented Oct 15, 2018

Description

Most of this PR consists of refactorings that don't change functionality.
The motivation for this PR is to clean up parts of the code to make it easier to add the 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 '--skip Address'

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

The --skip option can be used to temporary skip a test set.
Here is a list of all HSpec options.
This version supports all HSpec 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)

Copy link
Contributor

@KtorZ KtorZ left a comment

Choose a reason for hiding this comment

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

One important comment shouldn't be discarded. Also, please make sure the commit message contains the various points you've exposed in the PR's description.

@KtorZ
Copy link
Contributor

KtorZ commented Oct 15, 2018

Also, I've removed review requests for Ante, Denis and Pawel. No need to ping and disturb 4 persons for some minor cleanup 👍

@MarcFontaine MarcFontaine force-pushed the mafo/CO-424-CHW-tests branch 3 times, most recently from d47f7c9 to fd67505 Compare October 16, 2018 10:45
* [CO-424] new module SetupTestEnv.hs.
  Exports the function setupClient :: CLIOptions -> IO (WalletClient IO, Manager)
  which has been in-lined in main :: IO () before.

* [CO-424] Module Functions.hs has been renamed to RandomStateWalk.hs.

* [CO-424] Cleanups in Main.hs :
  Definitions related to the random state walk move to RandomStateWalk.hs

* [CO-424] Add a new CLI option --hspec-options to pass options to HSpec.
  This supersedes --match and --seed.
  Example : launch_integration_tests --hspec-options '--skip Address'
@MarcFontaine MarcFontaine force-pushed the mafo/CO-424-CHW-tests branch from fd67505 to 64092a7 Compare October 17, 2018 12:24
@MarcFontaine MarcFontaine merged commit dc7c025 into develop Oct 17, 2018
@KtorZ KtorZ deleted the mafo/CO-424-CHW-tests branch October 17, 2018 13:09
@KtorZ
Copy link
Contributor

KtorZ commented Oct 17, 2018

Note: There was still an integration test failing, have we looked into that:


  integration/TransactionSpecs.hs:102:22: 
  1) Transactions asset-locked wallets can receive funds and transactions are confirmed in index
       Falsifiable (after 1 test):
       
       not expected: 0

?


-- | Output for @Text@.
printT :: Text -> IO ()
printT = putStrLn
Copy link
Contributor

Choose a reason for hiding this comment

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

@MarcFontaine just as a side note, there is putText from Universum which does exactly this 😉

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thx yes putText should be used instead... I don't know where printT comes
from I was just moving snippets from one module to the other..much more should be done though

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.

3 participants