Skip to content
This repository has been archived by the owner on Apr 17, 2019. It is now read-only.

Tests MSVC build #2022

Merged
merged 5 commits into from Jan 29, 2019
Merged

Conversation

lebdron
Copy link
Contributor

@lebdron lebdron commented Jan 15, 2019

Description of the Change

Enable tests build with MSVC

  • Find gtest with installed config file
  • More ciso646 includes
  • Fix ODR violation in CryptoModelSigner
  • Change ip address from 0.0.0.0 to 127.0.0.1 for clients, since it is not possible to connect to 0.0.0.0
  • Add 5 second failure timeout in YacNetworkTest
  • Initialize Round struct in OnDemandConnectionManagerTest

Major change:

  • Move mocks to separate files to avoid the following errors:
    gtest/gtest-printers.h(157): error C2027: use of undefined type 'shared_model::interface::Domain' in torii_queries_test, where shared_model::interface::Domain is not used.

Benefits

Run tests and develop Iroha natively on Windows!

Possible Drawbacks

Some tests fail. Fixing them may require more changes, but this pull request is already big enough.

Usage Examples or Tests

Build

  • choco install visualstudio2017-workload-vctools - Microsoft compilers
  • choco install cmake - CMake
  • Install vcpkg as described from the following pull request [soci] Add postgresql feature microsoft/vcpkg#5029
  • Install the packages: vcpkg install protobuf:x64-windows grpc:x64-windows tbb:x64-windows gtest:x64-windows gflags:x64-windows soci[boost,postgresql]:x64-windows boost-filesystem:x64-windows boost-system:x64-windows boost-thread:x64-windows boost-variant:x64-windows boost-multiprecision:x64-windows boost-bimap:x64-windows boost-format:x64-windows boost-circular-buffer:x64-windows boost-assign:x64-windows boost-uuid:x64-windows boost-accumulators:x64-windows boost-property-tree:x64-windows boost-process:x64-windows
    Note the additional boost-uuid:x64-windows boost-accumulators:x64-windows boost-property-tree:x64-windows boost-process:x64-windows libraries compared to the list in PartialEq and PartialOrd of Identifiable structures  hyperledger/iroha#1988
  • cmake -HC:\path\to\iroha -BC:\path\to\build -DCMAKE_TOOLCHAIN_FILE=C:\path\to\vcpkg\scripts\buildsystems\vcpkg.cmake -G "Visual Studio 15 2017 Win64" -T host=x64
  • cmake --build C:\path\to\build

Run tests

  • cd C:\path\to\build
  • ctest -C Debug

Signed-off-by: Andrei Lebedev <lebdron@gmail.com>
Copy link
Contributor

@nickaleks nickaleks left a comment

Choose a reason for hiding this comment

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

Not able to test on Windows, but changes seem reasonable.

@@ -28,6 +31,17 @@ using ::testing::ByMove;
using ::testing::DefaultValue;
using ::testing::Return;

/**
* Factory for generation mock mutable storages.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
* Factory for generation mock mutable storages.
* Factory for mock mutable storage generation.

@@ -28,6 +31,17 @@ using ::testing::ByMove;
using ::testing::DefaultValue;
using ::testing::Return;

/**
* Factory for generation mock mutable storages.
* This method provide technique,
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
* This method provide technique,
* This method provides technique,

Copy link
Contributor

Choose a reason for hiding this comment

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

I understand that this is not your code, but since we move it anyway, might as well correct it.

@l4l l4l added needs-review pr awaits review from maintainers windows labels Jan 16, 2019
@lebdron lebdron changed the base branch from dev to develop January 18, 2019 08:41
Copy link
Contributor

@igor-egorov igor-egorov left a comment

Choose a reason for hiding this comment

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

Tests can be built and run.

Don't forget to set max_prepared_transactions=100 for Postgres

The following tests FAILED:
	  1 - module_ametsuchi_test (Failed)
	  7 - module_postgres_executor_test (Failed)
	  8 - module_postgres_query_executor_test (Failed)
	 28 - module_server_runner_test (Failed)
	 35 - module_json_command_converter_test (Failed)
	 36 - module_json_transaction_converter_test (Failed)
	 38 - module_json_query_factory_test (Exit code 0xc0000409
)
	 43 - module_mst_processor_test (Failed)
	 46 - module_block_loader_test (Failed)
	 57 - module_torii_transport_command_test (Failed)
	152 - integration_consensus_sunny_day (Failed)

@l4l l4l removed the needs-review pr awaits review from maintainers label Jan 19, 2019
@neewy
Copy link
Contributor

neewy commented Jan 23, 2019

@lebdron please resolve the conflicts and check if the tests can still build on Windows (attach the ctest output)

Signed-off-by: Andrei Lebedev <lebdron@gmail.com>
Signed-off-by: Andrei Lebedev <lebdron@gmail.com>
@lebdron
Copy link
Contributor Author

lebdron commented Jan 24, 2019

The following tests FAILED:
          1 - module_ametsuchi_test (Failed)
          7 - module_postgres_executor_test (Failed)
          8 - module_postgres_query_executor_test (Failed)
         28 - module_server_runner_test (Failed)
         35 - module_json_command_converter_test (Failed)
         36 - module_json_transaction_converter_test (Failed)
         38 - module_json_query_factory_test (Failed)
         43 - module_mst_processor_test (Failed)
         46 - module_block_loader_test (Failed)
         57 - module_torii_transport_command_test (Failed)
        142 - integration_consensus_sunny_day (Failed)

However, I had another set of failing tests before merging develop.

The following tests FAILED:
          1 - module_ametsuchi_test (Failed)
          7 - module_postgres_executor_test (Failed)
          8 - module_postgres_query_executor_test (Failed)
         28 - module_server_runner_test (Failed)
         35 - module_json_command_converter_test (Failed)
         36 - module_json_transaction_converter_test (Failed)
         38 - module_json_query_factory_test (Failed)
         43 - module_mst_processor_test (Failed)
         46 - module_block_loader_test (Failed)
         57 - module_torii_transport_command_test (Failed)
        145 - integration_add_signatory_test (Failed)
        147 - integration_get_asset_info_test (Failed)
        152 - integration_consensus_sunny_day (Failed)

Therefore integration tests should be checked for nondeterministic behavior in another task.

Signed-off-by: Andrei Lebedev <lebdron@gmail.com>
Signed-off-by: Andrei Lebedev <lebdron@gmail.com>
@lebdron lebdron requested a review from kamilsa January 29, 2019 05:39
Copy link
Contributor

@MBoldyrev MBoldyrev left a comment

Choose a reason for hiding this comment

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

I did not review this PR, but while it has collected 2 approves before one of them become invalid due to maintainer status loss, I just put my approve to fulfill the merge rules.

@lebdron lebdron merged commit 9a3a796 into hyperledger-archives:develop Jan 29, 2019
@lebdron lebdron deleted the feature/win-tests-build branch January 29, 2019 10:36
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Development

Successfully merging this pull request may close these issues.

None yet

6 participants