Conversation
richvdh
left a comment
There was a problem hiding this comment.
some suggestions which might stem entirely from my lack of familiarity with docker, or which might be useful.
I'm not sure if we've got the sytest/synapse divide quite right, but let's ignore that for now.
docker/Dockerfile
Outdated
| ENV LANGUAGE en_US:en | ||
| ENV LC_ALL en_US.UTF-8 | ||
|
|
||
| # The dockerfile context, when ran by the buildscript, will actually be the |
docker/synapse_sytest.sh
Outdated
|
|
||
| echo "Trying to get same-named sytest..." | ||
| wget -q https://github.com/matrix-org/sytest/archive/$branch_name.tar.gz -O sytest.tar.gz | ||
| 4 |
docker/README.md
Outdated
| ## Using the Synapse containers | ||
|
|
||
| Alternatively: | ||
| Once pulled from Docker Hub, the container can be ran on a Synapse checkout: |
| ``` | ||
| ./run-tests.pl | ||
| ``` | ||
| - matrixdotorg/sytest, a base container with SyTest dependencies installed |
There was a problem hiding this comment.
one thing that I didn't quite grok at first was that the docker images don't in fact include Sytest itself, which will be pulled from git. I realise it's alluded to below, but it might be worth making explicit?
docker/README.md
Outdated
| Where `<command>` is `./run-tests.pl` or similar. | ||
| This will run on the same branch in SyTest as the Synapse checkout, if possible, but will fall back to using develop. | ||
|
|
||
| If you want to use a checkout of SyTest, mount it to `/test` inside the container by adding `-v /path/to/sytest\:/test` to the docker command. |
There was a problem hiding this comment.
s/a checkout/an existing checkout/ ?
| @@ -0,0 +1,59 @@ | |||
| #! /usr/bin/env bash | |||
|
|
|||
| set -x | |||
There was a problem hiding this comment.
looks like we're going to need set -e here too.
There was a problem hiding this comment.
(with some faffery around the wget)
|
|
||
| # The dockerfile context, when ran by the buildscript, will actually be the | ||
| # repo root, not the docker folder | ||
| ADD install-deps.pl ./install-deps.pl |
There was a problem hiding this comment.
having added these to the docker image, are they ever actually used?
|
|
||
| set -x | ||
|
|
||
| if [ -e "/test/run-tests.pl" ] |
There was a problem hiding this comment.
why is this this /test/run-tests.pl rather than ./run-tests.pl ? The inconsistency makes me wonder if I'm misunderstanding what is going on. Which is entirely plausible, given my level of docker fu.
.gitignore
Outdated
| /var | ||
| *~ | ||
| \#* | ||
| logs/ |
There was a problem hiding this comment.
can we make these /logs and /results.tap rather than matching those filenames anywhere?
.gitignore
Outdated
| /.synapse-base | ||
| /synapse | ||
| /var | ||
| *~ |
There was a problem hiding this comment.
there's some attempt to sort this file; this and the below probably want to go alongside .*.swp
No description provided.