search

exercism / elm-test-runner

GNU Affero General Public License v3.0
3 stars 5 forks source link

Add scripts to run all tests in exercism/elm repo #43

Closed jiegillet closed 1 year ago

jiegillet commented 1 year ago

With these scripts, it will be possible to run elm-test-runner on each exercise from the exercism/elm repo in the CI, as discussed in this issue.

To test locally bin/run-all-exercises-in-docker.sh path/to/elm/repo.

When I try it, it's very slow and I get a bunch of "tinyproxy: Could not create listening sockets." so maybe the tinyproxy script needs to be modified, but I would like to see the behavior in the CI here before touching it.

mpizenberg commented 1 year ago

I can't check today but here are couple infos I remember.

First, Tiny proxy is used in the container to create a proxy server that automatically fail all http requests. This is the only I found to prevent the elm compiler to wait for roughly 5 seconds doing nothing when on Linux without network.

From memory, we do not check whether the proxy port is available, just assume it is. So I think I already had that tiny proxy error message in a context where I try to run it multiple times, resulting in the first time succeeding and all subsequent ones to fail due to occupied port. So the error might not be problematic.

Then regarding the long runtime duration. I cannot say exactly before 'ooking at it. One thing for sure is that due to the cost of starting Node, it is much more expensive to run tests one after each other, than to craft an executable concatenating all tests as done in CI of our main repo.

If you're running independant docker calls per test run. The cost per run is probably around 1s.

On Sat, Nov 12, 2022, 15:13 Jie @.***> wrote:

With these scripts, it will be possible to run elm-test-runner on each exercise from the exercism/elm repo in the CI, as discussed in this issue https://github.com/exercism/elm/issues/412.

To test locally bin/run-all-exercises-in-docker.sh path/to/elm/repo.

When I try it, it's very slow and I get a bunch of "tinyproxy: Could not create listening sockets." so maybe the tinyproxy script needs to be modified, but I would like to see the behavior in the CI here before touching it.

You can view, comment on, or merge this pull request online at:

https://github.com/exercism/elm-test-runner/pull/43 Commit Summary

File Changes

(3 files https://github.com/exercism/elm-test-runner/pull/43/files)

Patch Links:

— Reply to this email directly, view it on GitHub https://github.com/exercism/elm-test-runner/pull/43, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAWFOCMU6NZGGBK2ZN4D7Z3WH6Q2NANCNFSM6AAAAAAR6K3S4M . You are receiving this because you are subscribed to this thread.Message ID: @.***>

jiegillet commented 1 year ago

Mmh... There are still "tinyproxy: Could not create listening sockets." messages in the smoke-tests, but they are still very fast, so I guess it's OK? Maybe the proxy isn't required when we run docker with --network none?

This is one of those times docker frustrates me because its whole job is to make behavior reproducible.

jiegillet commented 1 year ago

So I think I already had that tiny proxy error message in a context where I try to run it multiple times, resulting in the first time succeeding and all subsequent ones to fail due to occupied port. So the error might not be problematic. Then regarding the long runtime duration. I cannot say exactly before looking at it.

Ah you are right, it's probably the repeated use. Not a problem then.

One thing for sure is that due to the cost of starting Node, it is much more expensive to run tests one after each other, than to craft an executable concatenating all tests as done in CI of our main repo. If you're running independant docker calls per test run. The cost per run is probably around 1s.

Yes, I considered that, it makes sense when you just want to run the tests, but here we also want to make sure that the test code is extracted and parsed without issues (like we had that one time when tests had identical names). To do that, we have no choice but to run elm-test-runner on each folder.

I still don't know why it runs fast here and slow on my machine, but this is much less problematic than the opposite situation :)

jiegillet commented 1 year ago

@mpizenberg it would be great to have your review for this one

jiegillet commented 1 year ago

Thank you for the review and QA!

I am thinking now about an exercise that might require two solution files, in this case the script will break, so I will have to revisit it, maybe I can take the opportunity to look what I could do to make bin/run.sh more parallel friendly. I'll open an issue to remember this.

@exercism/maintainers-admin could I get an approval for this?

jiegillet commented 1 year ago

Closed in favor of https://github.com/exercism/elm/pull/527