Implement hs-lib@repl test as integration_test #1692

k1nkreet · 2022-02-10T10:31:06Z

Since #1645 is merged we can gradually implement integration tests from RunTests.hs as integration_test rules. Here we have hs-lib@repl test scenario reimplemented this way

aherrmann · 2022-02-11T09:10:45Z

@k1nkreet It looks like CI is failing with a couple instances of

       ERROR: The Build Event Protocol upload failed: All retry attempts failed. DEADLINE_EXCEEDED: DEADLINE_EXCEEDED: deadline exceeded after 14.999828601s. [remote_addr=cloud.buildbuddy.io/34.82.173.239:443] DEADLINE_EXCEEDED: DEADLINE_EXCEEDED: deadline exceeded after 14.999828601s. [remote_addr=cloud.buildbuddy.io/34.82.173.239:443]`

Maybe worth asking on BuildBuddy Slack for some help.

k1nkreet · 2022-02-11T09:29:38Z

There was a discussion on the BuildBuddy slack about month ago about exactly the same issue occurred when "running multiple bazel invocations one after the other in github actions". Here is the fixes proposed in the thread:

If so running either bazel shutdown before the command - or setting the flag --keep_backend_build_event_connections_alive=false  would likely help

Another thing that might help (that I've seen fix other github actions related networking issues) is running this before the invocation in your github action:
sudo sysctl -w net.ipv4.tcp_keepalive_time=60

Either that or setting the --keep_backend_build_event_connections_alive=false Bazel flag are two options I'd try out.

And for the question "is --keep_backend_build_event_connections_alive=false affects builds performance" the answer was given:

Pretty much no impact - just starts a new BES connection on the second invocation instead of trying to reuse the old one. Just a one time thing and not super expensive.

aherrmann · 2022-02-11T09:34:49Z

@k1nkreet Thank you for digging into this. Yes, either option is worth a shot.

k1nkreet · 2022-02-11T09:44:57Z

https://github.com/tweag/rules_haskell/blob/master/.bazelrc#L67

It looks like we've already tried to fix exactly same problem with the second solution :)

#1692 (comment)

aherrmann · 2022-02-11T12:49:19Z

.github/workflows/workflow.yaml

@@ -50,6 +50,7 @@ jobs:
          nix-shell --arg docTools false --pure --run '
            set -euo pipefail
            ./tests/run-start-script.sh --use-nix
+            bazel shutdown
            bazel build //tests:run-tests
            ./bazel-ci-bin/tests/run-tests


One issue might be that tests/run-tests runs a couple Bazel commands as well.

…ng to" This reverts commit 82db9bc.

according to #1692 (comment)

did (#1486)

k1nkreet · 2022-02-15T09:19:44Z

I've found out this test wasn't supposed to be passing on macos-bindist because hs-lib depend on c2hs target and tagged with dont_test_on_darwin_with_bindist. So I've used this tag for hs_lib_repl_test integration test as well.

aherrmann

Good work, thank you!
A couple minor comments. Feel free to merge once addressed.

aherrmann · 2022-02-15T09:37:34Z

.github/workflows/workflow.yaml

@@ -29,6 +29,7 @@ jobs:
        env:
          BUILDBUDDY_API_KEY: ${{ secrets.BUILDBUDDY_API_KEY }}
        run: |
+          [[ ${{ runner.os }} == Linux ]] && sudo sysctl -w net.ipv4.tcp_keepalive_time=60


Suggested change

[[ ${{ runner.os }} == Linux ]] && sudo sysctl -w net.ipv4.tcp_keepalive_time=60

# Avoid failures of the form `deadline exceeded after 14999958197ns DEADLINE_EXCEEDED`.

# See https://github.com/tweag/rules_haskell/issues/1498 and https://github.com/tweag/rules_haskell/pull/1692.

[[ ${{ runner.os }} == Linux ]] && sudo sysctl -w net.ipv4.tcp_keepalive_time=60

aherrmann · 2022-02-15T09:38:00Z

.github/workflows/workflow.yaml

@@ -77,6 +78,7 @@ jobs:
        env:
          BUILDBUDDY_API_KEY: ${{ secrets.BUILDBUDDY_API_KEY }}
        run: |
+          sudo sysctl -w net.ipv4.tcp_keepalive_time=60


Suggested change

sudo sysctl -w net.ipv4.tcp_keepalive_time=60

# Avoid failures of the form `deadline exceeded after 14999958197ns DEADLINE_EXCEEDED`.

# See https://github.com/tweag/rules_haskell/issues/1498 and https://github.com/tweag/rules_haskell/pull/1692.

sudo sysctl -w net.ipv4.tcp_keepalive_time=60

aherrmann · 2022-02-15T09:38:17Z

.github/workflows/workflow.yaml

@@ -121,6 +123,7 @@ jobs:
          BUILDBUDDY_API_KEY: ${{ secrets.BUILDBUDDY_API_KEY }}
        shell: bash
        run: |
+          [[ ${{ runner.os }} == Linux ]] && sudo sysctl -w net.ipv4.tcp_keepalive_time=60


Suggested change

[[ ${{ runner.os }} == Linux ]] && sudo sysctl -w net.ipv4.tcp_keepalive_time=60

# Avoid failures of the form `deadline exceeded after 14999958197ns DEADLINE_EXCEEDED`.

# See https://github.com/tweag/rules_haskell/issues/1498 and https://github.com/tweag/rules_haskell/pull/1692.

[[ ${{ runner.os }} == Linux ]] && sudo sysctl -w net.ipv4.tcp_keepalive_time=60

aherrmann · 2022-02-15T09:40:34Z

tests/repl-targets/hs_lib_repl_test.go

+    },
+    local_snapshot = "@rules_haskell//:stackage_snapshot.yaml",
+    packages = [
+	"array",


Nit: Looks like there is some inconsistent indentation.

aherrmann · 2022-02-15T09:41:09Z

tests/repl-targets/hs_lib_repl_test.go

+        ],
+    },
+    local_snapshot = "@rules_haskell//:stackage_snapshot.yaml",
+    packages = [


Do we really need all these packages, or could this be slimmed down a bit?

aherrmann · 2022-02-15T09:41:56Z

tests/repl-targets/hs_lib_repl_test.go

+func TestMain(m *testing.M) {
+	it.TestMain(m, `
+-- WORKSPACE --
+load("@bazel_tools//tools/build_defs/repo:http.bzl", "http_archive")


Perhaps some of this boilerplate could be factored out?
Doesn't have to be in this PR, but, once we see what's common across these tests, it would make sense to factor that out.

workflow.yaml

Ilya Polyakovskiy added 2 commits February 10, 2022 13:28

add hs_lib_repl_test implemented as integration_test

2782e82

make hs_lib_repl_test size large

7b98a54

added bazel shutdown before bazel running tasks on ci according to

82db9bc

#1692 (comment)

aherrmann reviewed Feb 11, 2022

View reviewed changes

Ilya Polyakovskiy added 4 commits February 11, 2022 16:38

Revert "added bazel shutdown before bazel running tasks on ci accordi…

369ba69

…ng to" This reverts commit 82db9bc.

set net.ipv4.tcp_keepalive_time=60 before bazel invokation on ci

3f4d4b8

according to #1692 (comment)

set net.ipv4.tcp_keepalive_time=60 only on Linux runners

71f5465

don't test hs-lib@repl on macos-bindist and windows because they never

55f2c1c

did (#1486)

aherrmann approved these changes Feb 15, 2022

View reviewed changes

Ilya Polyakovskiy added 3 commits February 15, 2022 17:54

Add comment regarding setting net.ipv4.tcp_keepalive_time=60 in

65ff7be

workflow.yaml

fix indentation

4ecfff8

remove as much as possible from stack_snapshot is hs_lib_repl_test

489be7d

k1nkreet merged commit f53c6f0 into master Feb 15, 2022

k1nkreet deleted the ip/hs-lib-go-bazel-test branch February 15, 2022 20:47

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Implement hs-lib@repl test as integration_test #1692

Implement hs-lib@repl test as integration_test #1692

k1nkreet commented Feb 10, 2022 •

edited

Loading

aherrmann commented Feb 11, 2022

k1nkreet commented Feb 11, 2022 •

edited

Loading

aherrmann commented Feb 11, 2022

k1nkreet commented Feb 11, 2022

aherrmann Feb 11, 2022

k1nkreet commented Feb 15, 2022

aherrmann left a comment

aherrmann Feb 15, 2022

aherrmann Feb 15, 2022

aherrmann Feb 15, 2022

aherrmann Feb 15, 2022

aherrmann Feb 15, 2022

aherrmann Feb 15, 2022

Implement hs-lib@repl test as integration_test #1692

Implement hs-lib@repl test as integration_test #1692

Conversation

k1nkreet commented Feb 10, 2022 • edited Loading

aherrmann commented Feb 11, 2022

k1nkreet commented Feb 11, 2022 • edited Loading

aherrmann commented Feb 11, 2022

k1nkreet commented Feb 11, 2022

aherrmann Feb 11, 2022

Choose a reason for hiding this comment

k1nkreet commented Feb 15, 2022

aherrmann left a comment

Choose a reason for hiding this comment

aherrmann Feb 15, 2022

Choose a reason for hiding this comment

aherrmann Feb 15, 2022

Choose a reason for hiding this comment

aherrmann Feb 15, 2022

Choose a reason for hiding this comment

aherrmann Feb 15, 2022

Choose a reason for hiding this comment

aherrmann Feb 15, 2022

Choose a reason for hiding this comment

aherrmann Feb 15, 2022

Choose a reason for hiding this comment

k1nkreet commented Feb 10, 2022 •

edited

Loading

k1nkreet commented Feb 11, 2022 •

edited

Loading