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.

Sign up for GitHub

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

Jump to bottom

Relocate reusable integration tests components to core #396

Merged
merged 4 commits into from
Jun 11, 2019
Merged

Relocate reusable integration tests components to core #396

merged 4 commits into from
Jun 11, 2019

Conversation

paweljakubas
Copy link
Contributor

Issue Number

#358

Overview

  • I have relocated integration test components from http-bridge to core and divided Faucet

Comments

@paweljakubas paweljakubas requested review from KtorZ and piotr-iohk June 11, 2019 09:12
@KtorZ KtorZ mentioned this pull request Jun 11, 2019
Closed
3 tasks
Anviking
Anviking reviewed Jun 11, 2019
View reviewed changes
Copy link
Member

@Anviking Anviking left a comment

Choose a reason for hiding this comment

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

lgtm!

stack.yaml Outdated Show resolved Hide resolved
lib/http-bridge/cardano-wallet-http-bridge.cabal Outdated
@@ -65,6 +65,7 @@ library
Cardano.Wallet.HttpBridge.Transaction
Data.Packfile
Servant.Extra.ContentTypes
Paths_cardano_wallet_http_bridge
Copy link
Member

Choose a reason for hiding this comment

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

Unrelated, but had no idea this was possible 😮

@Anviking Anviking force-pushed the paweljakubas/358/relocate-integration-tests branch from 13f9966 to 1e1ae11 Compare June 11, 2019 11:44
@Anviking
Copy link
Member

I rebased and removed unused LambdaCase in http-bridge/test/integration/Cardano/Faucet.hs to satisfy failing hlint.

KtorZ
KtorZ reviewed Jun 11, 2019
View reviewed changes
lib/core/test/integration/cardano-wallet-core-integration.cabal Outdated
@@ -0,0 +1,68 @@
name: cardano-wallet-core-integration
Copy link
Member

@KtorZ KtorZ Jun 11, 2019 •
edited
Loading

Choose a reason for hiding this comment

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

Let's not make it a library, but have it simply located in core/test and referred to in the corresponding integration tests using hs-source-dirs. Having test code as a library will have further complications.

Copy link
Contributor Author

@paweljakubas paweljakubas Jun 11, 2019 •
edited
Loading

Choose a reason for hiding this comment

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

good idea, addressed in new commit

@paweljakubas paweljakubas requested a review from KtorZ June 11, 2019 13:25
@paweljakubas paweljakubas force-pushed the paweljakubas/358/relocate-integration-tests branch 3 times, most recently from 5af5819 to 586d1b5 Compare June 11, 2019 14:58
@paweljakubas
Copy link
Contributor Author

I have weeder complaining :

== Section test:integration ==
Missing other-modules entry
* Test.Integration.Faucet
* Test.Integration.Framework.DSL
* Test.Integration.Framework.Request
* Test.Integration.Scenario.API.Addresses
* Test.Integration.Scenario.API.Transactions
* Test.Integration.Scenario.API.Wallets
* Test.Integration.Scenario.CLI.Addresses
* Test.Integration.Scenario.CLI.Mnemonics
* Test.Integration.Scenario.CLI.Transactions
* Test.Integration.Scenario.CLI.Wallets
Module not compiled
* Test.Integration.Faucet
* Test.Integration.Framework.DSL
* Test.Integration.Framework.Request
* Test.Integration.Framework.TestData
* Test.Integration.Scenario.API.Addresses
* Test.Integration.Scenario.API.Transactions
* Test.Integration.Scenario.API.Wallets
* Test.Integration.Scenario.CLI.Addresses
* Test.Integration.Scenario.CLI.Mnemonics
* Test.Integration.Scenario.CLI.Transactions
* Test.Integration.Scenario.CLI.Wallets
Redundant build-depends entry
* aeson-qq
* command
* cryptonite
* exceptions
* generic-lens
* hspec-expectations-lifted
* http-api-data
* http-types
* template-haskell

KtorZ
KtorZ approved these changes Jun 11, 2019
View reviewed changes
Copy link
Member

@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.

👍

@paweljakubas
Copy link
Contributor Author

I wonder what to do with weeder - refactor cabal or add appropriate section to .weeder.yaml?

@paweljakubas paweljakubas force-pushed the paweljakubas/358/relocate-integration-tests branch from 586d1b5 to f4dda52 Compare June 11, 2019 15:59
@KtorZ KtorZ force-pushed the paweljakubas/358/relocate-integration-tests branch from f4dda52 to a2b1246 Compare June 11, 2019 17:10
@KtorZ KtorZ merged commit 8820524 into master Jun 11, 2019
@KtorZ KtorZ deleted the paweljakubas/358/relocate-integration-tests branch June 11, 2019 17:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Anviking Anviking Anviking left review comments

@KtorZ KtorZ KtorZ approved these changes

@piotr-iohk piotr-iohk Awaiting requested review from piotr-iohk

Assignees

@paweljakubas paweljakubas

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@paweljakubas @Anviking @KtorZ