Change integration tests implementation to one based on `rules_bazel_integration_test` #1766

k1nkreet · 2022-06-23T23:43:08Z

This PR suggests alternative approach to integration testing, based on rules_bazel_integration_test ruleset. Unfortunately I could not come up with anything less radical than put all required changes in one PR, cause otherwise both implementation would stay for a while in repo which could be very unclear and misleading

So here is a list of changes I've put here:

Create haskell_bazel_integration_test rule for integration test
with test scenario written in haskell and similar to go_bazel_test setup procedure
Create rules_haskell_integration_test macro for integration test running
haskell scenario with all bazel versions supported in rules_haskell
Remove rules_go-based integration_test implementation
Remove rules_go patching for the sake of this implementation
Create IntegrationTesting library with helper functions formerly
located in RunTests.hs
Reimplement existing integration_test implementations with
rules_haskell_integration_test

…sts: * create haskell_bazel_integration_test rule for integration test with test scenario written in haskell and similar to go_bazel_test setup procedure * create rules_haskell_integration_test macro for integration test running haskell scenario with all bazel versions supported in rules_haskell * remove rules_go-based integration_test implementation * remove rules_go patching for the sake of this implementation * create IntegrationTesting library with helper functions formerly located in RunTests.hs * Reimplement existing integration_test implementations with rules_haskell_integration_test

googleson78 · 2022-06-24T07:04:55Z

Awesome!

Regarding 6., I added a couple more tests using the old framework in tests/haskell_module/repl, so you might have to modify those as well.

tests/integration_testing/BUILD.bazel

rules_haskell_integration_test

googleson78 · 2022-06-24T12:42:19Z

Thanks! Sorry for creating additional work for the migration.

k1nkreet · 2022-06-24T13:06:45Z

Thanks! Sorry for creating additional work for the migration.

There is plenty of it still in RunTests.hs, so don't be :)

integration testing, put those dependencies downloading in tests/integration_testing/dependencies.bzl

rules_bazel_integration_test, removed redundant set of TMPDIR

rules_haskell_integration_test, fix buildifier formatting

tests to run bazel with it

k1nkreet · 2022-06-27T22:56:53Z

Tests on Windows are failing due to the bug in rules_bazel_integration_test. I'm going to help with debug and fixing it. In the meantime we can ignore these tests on windows until it fixed, or just wait for the fix

aherrmann

Thanks a lot for pushing this forward! This looks a lot more pleasant to use!

A couple comments/questions.

I'm going to help with debug and fixing it. In the meantime we can ignore these tests on windows until it fixed, or just wait for the fix

Thanks! Happy to temporarily skip those on Windows. You could already line up a PR to re-enable them so that it's not forgotten.

tests/integration_testing/IntegrationTesting.hs

.bazelrc

shell.nix

rules_haskell's bazelrc

aherrmann

Thank you, that looks great!

I fixed a couple typos in the README, please take another look in case I messed something up.

k1nkreet · 2022-07-01T13:38:54Z

Thank you, that looks great!

I fixed a couple typos in the README, please take another look in case I messed something up.

Thanks! Btw there is a problem with Cross ci job. It complains on "no space left on device" for some Nix operations. Do you have an idea how we could fix it?

aherrmann · 2022-07-01T14:43:12Z

Btw there is a problem with Cross ci job. It complains on "no space left on device" for some Nix operations. Do you have an idea how we could fix it?

Hmm, this is odd. It seems to be failing when fetching haskell.nix already.

unpacking 'https://github.com/input-output-hk/hackage.nix/archive/3bce0587d9ffe56d11e83b5eeccd63e49b643bdd.tar.gz'...
tar: hackage/sloane-1.8.2-r0-26781c13cac7e1a8a70fb3928e39a4b6bac5b6ea742f83ddc174a3b915ba8207.nix: Cannot write: No space left on device

I'm not sure what's causing this. AFAIK nothing in this PR should affect that pipeline, right?

k1nkreet · 2022-07-01T14:54:36Z

I'm not sure what's causing this. AFAIK nothing in this PR should affect that pipeline, right?

Yes, and it was passing before README fixes, which I hardly believe could influence this

install some nix dependencies

k1nkreet · 2022-07-05T10:00:45Z

I removed setting TMPDIR=/tmp from shell.nix cause it was a hack needed by previous implementation of integration testing, but it turned out that now nix-shell could not install it's dependencies sometimes because standard Nix TMPDIR has not enough space. So I returned this setting back for now

…newer version where dependency on clang is removed * Remove clang dependency in shell.nix

its possible now in rules_bazel_integration_test

k1nkreet · 2022-07-05T15:42:56Z

Since bazel-contrib/rules_bazel_integration_test#71 is merged, there is an ability to specify the env for integration test. So I've changed a way how nixpkgs indication get passed into the test from command line to environment. Among other things, It allows to wrap integration tests with hspec again as it was done in RunTests.hs and which I removed because it was impossible to use command line with it. I was going to return it back, but I'm doubting if it's actually useful in this scenario (when every test is a separate executable running probably one function). @aherrmann Do you have an opinion?

aherrmann · 2022-07-06T07:51:30Z

@k1nkreet I'd say, what's important is that test failures produce equally clear and understandable error messages. Otherwise, as you say, if each binary represents a single test anyway, then there isn't much benefit in wrapping it in hspec.

k1nkreet · 2022-07-07T15:11:32Z

@aherrmann Can we add this to the merge queue, or there is something else to fix?

aherrmann

Looks good to merge. 🎉

googleson78 reviewed Jun 24, 2022

View reviewed changes

tests/integration_testing/BUILD.bazel Outdated Show resolved Hide resolved

implement haskell_module integration tests as

89d190f

rules_haskell_integration_test

Ilya Polyakovskiy added 6 commits June 24, 2022 15:13

make buildifier happy

09ab1a8

enable fail_not_supported for nixpkgs_package bazel packages for

0bab548

integration testing, put those dependencies downloading in tests/integration_testing/dependencies.bzl

added clang dependency in shell.nix required by

aa139a5

rules_bazel_integration_test, removed redundant set of TMPDIR

use "is_nix_shell" to generate different targets in

4fa9f49

rules_haskell_integration_test, fix buildifier formatting

pass BAZEL_USE_CPP_ONLY_TOOLCHAIN environment variable into integration

3c3cfaf

tests to run bazel with it

don't run c2hs dependent integration test on macos-bindist an windows

c45459c

k1nkreet requested a review from aherrmann June 28, 2022 08:08

aherrmann reviewed Jun 28, 2022

View reviewed changes

Ilya Polyakovskiy and others added 8 commits June 28, 2022 15:28

make a list of bazel subcommands suitable for applying --config with

03df6f6

rules_haskell's bazelrc

added signatures for helper functions in IntegrationTesting

2d2d62b

add "dont_test_on_windows" to default tags for integration tests

61ee958

add signatures for global functions in IntegrationTesting

8f23668

add README for integration testing

5e16530

prevent rules_haskell_integration_test expanding on windows

56eaac4

remove redundant comment

cb6b036

typos

3b80768

aherrmann approved these changes Jul 1, 2022

View reviewed changes

set TMPDIR="/tmp" cause usual nix-shell tmp has not enough space to

23cfbed

install some nix dependencies

k1nkreet force-pushed the ip/bazel-integration-test branch from d90aea3 to 23cfbed Compare July 5, 2022 09:58

Ilya Polyakovskiy added 2 commits July 5, 2022 16:12

* Change bazel-contrib/rules_bazel_integration_test dependency to a …

04f752c

…newer version where dependency on clang is removed * Remove clang dependency in shell.nix

integration testing: pass nixpkgs info through env instead of args since

8b50dd6

its possible now in rules_bazel_integration_test

Ilya Polyakovskiy added 2 commits July 7, 2022 14:32

Wrap integration tests in hspec to improve readability of test outputs

44ff83b

add hspec usage in example in integration testing README

90a9ab5

aherrmann approved these changes Jul 7, 2022

View reviewed changes

aherrmann added the merge-queue merge on green CI label Jul 7, 2022

mergify bot merged commit 62a3583 into master Jul 7, 2022

mergify bot deleted the ip/bazel-integration-test branch July 7, 2022 15:25

mergify bot removed the merge-queue merge on green CI label Jul 7, 2022

k1nkreet mentioned this pull request Jul 11, 2022

Move as much of tests/RunTests.hs into bazel, remove if possible #951

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Change integration tests implementation to one based on `rules_bazel_integration_test` #1766

Change integration tests implementation to one based on `rules_bazel_integration_test` #1766

k1nkreet commented Jun 23, 2022

googleson78 commented Jun 24, 2022 •

edited

Loading

googleson78 commented Jun 24, 2022

k1nkreet commented Jun 24, 2022

k1nkreet commented Jun 27, 2022

aherrmann left a comment

aherrmann left a comment

k1nkreet commented Jul 1, 2022

aherrmann commented Jul 1, 2022

k1nkreet commented Jul 1, 2022

k1nkreet commented Jul 5, 2022

k1nkreet commented Jul 5, 2022 •

edited

Loading

aherrmann commented Jul 6, 2022

k1nkreet commented Jul 7, 2022

aherrmann left a comment

Change integration tests implementation to one based on rules_bazel_integration_test #1766

Change integration tests implementation to one based on rules_bazel_integration_test #1766

Conversation

k1nkreet commented Jun 23, 2022

googleson78 commented Jun 24, 2022 • edited Loading

googleson78 commented Jun 24, 2022

k1nkreet commented Jun 24, 2022

k1nkreet commented Jun 27, 2022

aherrmann left a comment

Choose a reason for hiding this comment

aherrmann left a comment

Choose a reason for hiding this comment

k1nkreet commented Jul 1, 2022

aherrmann commented Jul 1, 2022

k1nkreet commented Jul 1, 2022

k1nkreet commented Jul 5, 2022

k1nkreet commented Jul 5, 2022 • edited Loading

aherrmann commented Jul 6, 2022

k1nkreet commented Jul 7, 2022

aherrmann left a comment

Choose a reason for hiding this comment

Change integration tests implementation to one based on `rules_bazel_integration_test` #1766

Change integration tests implementation to one based on `rules_bazel_integration_test` #1766

googleson78 commented Jun 24, 2022 •

edited

Loading

k1nkreet commented Jul 5, 2022 •

edited

Loading