Fixed flakiness in integration tests

[blakehatch]

Added server readiness string listening to integration tests to reduce flakiness

Description

Listens for string from docker-compose logs to indicate that the servers are ready and listening before running integration tests.

Fixes #375

Type of change

• Bug fix (non-breaking change which fixes an issue)

How Has This Been Tested?

Ran all tests individually and together. TLS test was flaky when running with all tests before change and had no failed runs afterwards.

Checklist

• Updated documentation if needed
• Tests added/amended
• bazel test //... passes locally
• PR is contained in a single commit, using git amend see some docs

---

This change is 

[aaronmondal]

Reviewed 1 of 1 files at r1, all commit messages.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @blakehatch)

---

run_integration_tests.sh line 111 at r1 (raw file):

      echo "String 'Ready, listening on' not found in the logs within the given time."
    fi
    set +e

Hmm would there be a way to do this without the perl wrapper?

Also somewhat related question: WDYT about migrating these tests to rust in the long run? A tricky thing here seems to be getting cross-platform compatibility right. Rust might help with that.

[blakehatch]

Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @MarcusSorealheis)

---

run_integration_tests.sh line 111 at r1 (raw file):

Previously, aaronmondal (Aaron Siddhartha Mondal) wrote…

Hmm would there be a way to do this without the perl wrapper?

Also somewhat related question: WDYT about migrating these tests to rust in the long run? A tricky thing here seems to be getting cross-platform compatibility right. Rust might help with that.

I originally tried timeout 30 bash -c 'until sudo docker-compose logs | grep -q "Ready, listening on"; do sleep 1; done' which is cleaner and works on Linux but the integration tests recently had some functionality added to make it support MacOS so I decided to use a command that works on all Unix-like OSes.

@MarcusSorealheis and I were discussing converting these integration tests to Python instead of Bash to fix the cross-platform compatibility while making the translation from Bash a bit simpler than rust. I do agree high level that they should definitely be converted in some form to have better cross-platform compatibility than the currently do.

[aaronmondal]

I guess let's just do it this way then. I'm not a huge fan, but it doesn't seem to really matter if we plan to port this to some other language anyways 💃

Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @blakehatch)

[blakehatch]

Yeah agree not a huge fan either but best not to break functionality in the meantime, esp in the context of the likely switch as you said.🕺

Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @blakehatch)

[allada]

Reviewable status: all files reviewed, 1 unresolved discussion

---

run_integration_tests.sh line 105 at r1 (raw file):

    echo "Running test $FILENAME"
    sudo docker-compose up -d
    if perl -e 'alarm shift; exec @ARGV' 30 bash -c 'until sudo docker-compose logs | grep -q "Ready, listening on"; do sleep 1; done'    

nit: spaces at end of line.

[allada]

Reviewable status: all files reviewed, 2 unresolved discussions

---

run_integration_tests.sh line 105 at r1 (raw file):

Previously, allada (Nathan (Blaise) Bruer) wrote…

nit: spaces at end of line.

Btw, this could have been done with a bash for loop in a function.

function wait_for_ready() {
  for i in $(seq 1 10); do
    if sudo docker-compose logs | grep -q "Ready, listening on"; then
      return
    fi
    sleep 1
  done
  echo "Timed out"
  exit 1
}

wait_for_ready

---

run_integration_tests.sh line 109 at r1 (raw file):

      echo "String 'Ready, listening on' found in the logs."
    else
      echo "String 'Ready, listening on' not found in the logs within the given time."

nit: Also we don't exit fail here so this message will be buried deep in the logs.

[MarcusSorealheis]

It's merged and is totally fine. A small nit

In the future, I'd like to avoid Perl. There were some exceptions to this rule a few years ago, but I don't know if even those remain valid reasons to insist on using Perl. I would to need to look at some references.

Reviewable status: all files reviewed, 2 unresolved discussions