Fix test runner #22

2Grey · 2021-09-03T12:01:30Z

Fixes for #21

Fix default value for showSkipped flag
Fix creating output directory if doesn't exist
Fix run.sh script

Runs

Direct launch of TestRunner

./bin/TestRunner --slug freelancer-rates --solution-directory ./freelancer-rates --output-directory ./Out --swift-location $(which swift) --build-directory "/tmp"

Result

{
    "status" : "pass",
    "tests" : [
        {
            "name" : "FreelancerRatesTests.FreelancerRatesTests testdailyRateFrom",
            "status" : "pass"
        }
    ]
}

Script run of TestRunner

./bin/run.sh freelancer-rates ./freelancer-rates ./Out

Result

{
  "status" : "pass",
  "tests" : [
    {
      "name" : "FreelancerRatesTests.FreelancerRatesTests testdailyRateFrom",
      "status" : "pass"
    },
    {
      "name" : "FreelancerRatesTests.FreelancerRatesTests testmonthlyRoundDown",
      "status" : "pass"
    },
    {
      "name" : "FreelancerRatesTests.FreelancerRatesTests testmonthlyRoundUp",
      "status" : "pass"
    },
    {
      "name" : "FreelancerRatesTests.FreelancerRatesTests testworkdaysIn",
      "status" : "pass"
    },
    {
      "name" : "FreelancerRatesTests.FreelancerRatesTests testworkdaysShouldRoundDown",
      "status" : "pass"
    }
  ]
}
freelancer-rates: comparing ./Out/results
diff: ./freelancer-rates/freelancer-rates/results.json: No such file or directory

iHiD · 2021-09-03T12:04:06Z

Thanks @2Grey :)

@ErikSchierboom will take a look at this on Tuesday if no-one else gets to it first!

ErikSchierboom · 2021-09-07T12:29:17Z

.dockerignore

@@ -1 +1,2 @@
-test/output
+.github/
+tests/


The tests are passed to the test scripts via a volume mount, but they don't have to be present when building the Dockerfile. Having them here has the benefit of being able to change the tests without having to rebuild the Docker image.

ErikSchierboom · 2021-09-07T12:30:06Z

README.md

@@ -1,42 +1,51 @@
 # Exercism Swift Test Runner


I've updated this file to make it similar to existing test runner READMEs, which was needed because I change the test script files to be consistent too.

ErikSchierboom · 2021-09-07T12:34:35Z

bin/run-all-in-docker.sh

-# build docker image
-docker build --rm --no-cache -t swift-test-runner .
-
-docker run \


There were several options missing here, including a key one: --network none. Other test runners have this file named run-all-tests-in-docker, so I've renamed the file and added the right flags that match how the test runner is actually run on the website.

ErikSchierboom · 2021-09-07T12:35:49Z

bin/run-in-docker.sh

-    echo "usage: run-in-docker.sh exercise-slug ./relative/path/to/solution/folder/ ./relative/path/to/output/directory/"
+printHelpAndExit() {
+    echo -e "${ORANGE}USAGE${NC}"
+    echo -e "   ./run-in-docker.sh ${GREEN}-s${NC} two-fer ${GREEN}-i${NC} /absolute/path/to/solution/folder/ ${GREEN}-o${NC} ./absolute/path/to/output/directory/"


I've converted the relative paths to absolute paths as there is no guarantee that the path is relative (see https://github.com/exercism/docs/blob/main/building/tooling/test-runners/docker.md) and in fact it is more likely for it to be absolute.

ErikSchierboom · 2021-09-07T12:36:32Z

bin/run-in-docker.sh

 # build docker image
-docker build --rm --no-cache -t swift-test-runner .
+docker build --rm --no-cache -t exercism/swift-test-runner .


This is the actual name we use for the test runner, so adding the prefix ensure that the local development environment uses this built version.

ErikSchierboom · 2021-09-07T12:36:42Z

bin/run-in-docker.sh

+    --network none \
+    --read-only \
+    --mount type=bind,src="${INPUT_DIR}",dst=/solution \
+    --mount type=bind,src="${OUTPUT_DIR}",dst=/output \
+    --mount type=volume,dst=/tmp \


Add the proper flags here

ErikSchierboom · 2021-09-07T12:37:03Z

bin/run-tests-in-docker.sh

+    --network none \
+    --read-only \
+    --mount type=bind,src="${PWD}/tests",dst=/opt/test-runner/tests \
+    --mount type=volume,dst=/tmp \
+    --workdir /opt/test-runner \
+    --entrypoint /opt/test-runner/bin/run-tests.sh \


Added the proper flags here

ErikSchierboom · 2021-09-07T12:38:10Z

bin/run.sh

 # echo "-------------"
-RUNALL=true "${BASEDIR}"/TestRunner --slug "${SLUG}" --solution-directory "${INPUT_DIR}/${SLUG}" --output-directory "${OUTPUT_DIR}" --swift-location $(which swift) --build-directory "/tmp/"
+
+RUNALL=true "${BASEDIR}"/TestRunner --slug "${SLUG}" --solution-directory "${INPUT_DIR}" --output-directory "${OUTPUT_DIR}" --swift-location $(which swift) --build-directory "/tmp"


I've removed the /${SLUG} argument from the solution directory as the solution directory should be the argument that is passed in.

ErikSchierboom

Thanks so much for fixing the test runner. I've done a bit of work to get the test runner to adhere to the spec fully, but it has succeeded!

ErikSchierboom · 2021-09-07T13:38:36Z

.github/workflows/main.yml

-      - '.gitignore'
-      - 'LICENSE'
-      - '**.md'
+      - main


We renamed the branch to main

ErikSchierboom · 2021-09-07T14:19:32Z

.gitignore

 .DS_Store
 **/.build
 **/Packages
 **/Package.resolved
 **/*.xcodeproj
+bin/TestRunner


We usually don't store binaries in our repos

ErikSchierboom · 2021-09-07T14:20:20Z

bin/run.sh

+# $1: exercise slug
+# $2: absolute path to solution folder
+# $3: absolute path to output directory


I've changed this file to take positional arguments as that is how the interface requires it.

ErikSchierboom · 2021-09-07T14:23:27Z

Let me know if there is anything you want to know about the commits I added.

2Grey added 3 commits September 3, 2021 15:46

Fix default value for showSkipped flag

c99b41d

Fix creating output directory if doesn't exist

7bba9b6

Fix run.sh script

3bc84ef

2Grey requested a review from a team as a code owner September 3, 2021 12:01

rodiobe approved these changes Sep 3, 2021

View reviewed changes

2Grey and others added 12 commits September 5, 2021 20:47

Fix code merge error

96fd1fe

Add named arguments and color output for scripts

153c918

Swiftformat project

8793e70

Rename files

b85fa34

Remove docker-specific test results

af776a3

Rename test dir to tests

478a3df

Update test run scripts

fa2d947

Exclude tests from docker context

e8659b8

Remove comment from Dockerfile

afb12e0

Update README

c26404f

Use absolute paths

2aa689c

Update docker settings

b25bfc3

ErikSchierboom reviewed Sep 7, 2021

View reviewed changes

ErikSchierboom added 9 commits September 7, 2021 14:41

Fix running of tests

057985e

Add expected results files

0dab84b

Update CI script

74484d4

Remove extra content from output

1734e71

Remove unneeded code

2e36713

Add todo for readonly

a58351c

Fix paths

83957c1

Allow for caching

bd22d5a

Remove binary

0b378a7

Fix script to adhere to interface

4bd09a6

ErikSchierboom approved these changes Sep 7, 2021

View reviewed changes

ErikSchierboom added the x:size/large Large amount of work label Sep 7, 2021

ErikSchierboom merged commit 6d4b99c into exercism:main Sep 7, 2021

angelikatyborska mentioned this pull request Dec 8, 2021

remove TestRunner from bin #4

Closed

This was referenced Jul 14, 2022

Document reputation label exercism/docs#347

Merged

Consider correct reputation amounts for different labels exercism/exercism#6440

Open

Consider correct reputation amounts for different labels exercism/exercism#6441

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix test runner #22

Fix test runner #22

2Grey commented Sep 3, 2021 •

edited

Loading

iHiD commented Sep 3, 2021

ErikSchierboom Sep 7, 2021

ErikSchierboom Sep 7, 2021

ErikSchierboom Sep 7, 2021

ErikSchierboom Sep 7, 2021

ErikSchierboom Sep 7, 2021

ErikSchierboom Sep 7, 2021

ErikSchierboom Sep 7, 2021

ErikSchierboom Sep 7, 2021

ErikSchierboom left a comment

ErikSchierboom Sep 7, 2021

ErikSchierboom Sep 7, 2021

ErikSchierboom Sep 7, 2021

ErikSchierboom commented Sep 7, 2021

Fix test runner #22

Fix test runner #22

Conversation

2Grey commented Sep 3, 2021 • edited Loading

Fixes for #21

Runs

Direct launch of TestRunner

Result

Script run of TestRunner

Result

iHiD commented Sep 3, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ErikSchierboom left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ErikSchierboom commented Sep 7, 2021

2Grey commented Sep 3, 2021 •

edited

Loading