Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Tests fail with no output #1761

Open
per-oestergaard opened this issue Nov 1, 2023 · 15 comments
Open

Tests fail with no output #1761

per-oestergaard opened this issue Nov 1, 2023 · 15 comments

Comments

@per-oestergaard
Copy link

Hi

I have two Rust tests, Luhn and Forth, that both passes all test locally but fails online. In both cases, the output from the test are -
image

To troubleshoot, I cloned https://github.com/exercism/rust-test-runner and tests runs successfully locally using the rtr docker image build with bin/test.sh (or the record, I ran the tests using ../rust-test-runner/bin/test-exercise.sh /workspaces/exercism-rust/rust/forth

(the next observations are created using the luhn exercise, but I do not think it matters)

If I provoke a syntax error in the online editor, the compilation error is shown.
If I provoke a result error in the online editor, it do not get the error returned.
If I switch from regex = { version = "*"} in Cargo.toml, to version 1.10.2 (the latest), the build fails and I get a message. If I use version 1.10.0, "One of the tests timed out" or the "no result" happens.

Not sure what I can do more to troubleshoot the issue.

Copy link
Contributor

github-actions bot commented Nov 1, 2023

Hello. Thanks for opening an issue on Exercism. We are currently in a phase of our journey where we have paused community contributions to allow us to take a breather and redesign our community model. You can learn more in this blog post. As such, all issues and PRs in this repository are being automatically closed.

That doesn't mean we're not interested in your ideas, or that if you're stuck on something we don't want to help. The best place to discuss things is with our community on the Exercism Community Forum. You can use this link to copy this into a new topic there.


Note: If this issue has been pre-approved, please link back to this issue on the forum thread and a maintainer or staff member will reopen it.

@github-actions github-actions bot closed this as completed Nov 1, 2023
@senekor senekor reopened this Nov 1, 2023
@senekor
Copy link
Contributor

senekor commented Nov 1, 2023

@per-oestergaard Thanks for reporting.

The supported regex version is 1.9.1:
https://github.com/exercism/rust-test-runner/blob/main/local-registry/Cargo.toml#L148

So that's why pinning the version fails. I guess the dependabot config is borked, it's supposed to update these pinned versions every week. I suspect this is unrelated to the empty output.

I'm not yet sure what's more plausible, an issue with the test runner itself, which I might be able to fix, or just some infra issues.

I'll have to think a bit more about the next debugging step.

@senekor
Copy link
Contributor

senekor commented Nov 1, 2023

Does it happen reliably if you add more iterations with some whitespace changes?

@per-oestergaard
Copy link
Author

Does it happen reliably if you add more iterations with some whitespace changes?

Yes.

Thanks for coming back :)

I tried to specify 1.9.1, but that did not change anything (I normally just use * in Exercism as I haven't encountered any version dependencies yet)

So I could look like an infra issue.

@per-oestergaard
Copy link
Author

In my forth solution, I get this error when specifying regex 1.9.1

Unpacking aho-corasick v1.1.2 (registry /opt/test-runner/local-registry)
Unpacking either v1.9.0 (registry /opt/test-runner/local-registry)
Unpacking itertools v0.11.0 (registry /opt/test-runner/local-registry)
Unpacking memchr v2.6.4 (registry /opt/test-runner/local-registry)
Unpacking regex v1.10.2 (registry /opt/test-runner/local-registry)
Unpacking regex-automata v0.4.3 (registry /opt/test-runner/local-registry)
Unpacking regex-syntax v0.8.2 (registry /opt/test-runner/local-registry)
Compiling memchr v2.6.4
Compiling regex-syntax v0.8.2
Compiling aho-corasick v1.1.2
Compiling regex-automata v0.4.3
Compiling either v1.9.0
Compiling itertools v0.11.0
Compiling regex v1.10.2
Compiling forth v1.7.0 (/mnt/exercism-iteration)
timeout: sending signal TERM to command 'cargo'

note it is compiling regex 1.10.2.

@senekor
Copy link
Contributor

senekor commented Nov 1, 2023

Ooh, I think I get what's happening with the regex version. I suppose dependabot only opens PRs for breaking changes, since we don't have a lockfile committed.

If you can confirm that tests passed while running the docker image locally, and the same solution failed online, then it's probably an infra issue.

One more question: You said your experiments with the regex version were done on the luhn exercise. Are you also using dependencies in the forth exercise? Maybe even the same one? didn't read the last comment well enough, looks like both luhn and forth use regex. Do you also have problems with exercise solutions that don't use dependencies at all?

I guess it is possible that your local docker container behaves differently than the deployed one because the latter was built with presently outdated dependencies. I absolutely need to add a lockfile to the test runner.

@per-oestergaard
Copy link
Author

I have been debugging on the simple hello-world solution. I tried serde, itertools, time and regex. Only regex gives the problem. However, it seems like you have some "runners" and that some of those fails. I just added an extra space to this

[package]
edition = "2021"
name = "hello-world"
version = "1.1.0"

[dependencies]
regex = { version = "1.0" }   

And clicked 'run tests' again. It fails, fails, suddenly fails with a timeout error, but I also managed for it to succeed!! So bad "runners"? Or a timeout that is too short?

@senekor
Copy link
Contributor

senekor commented Nov 2, 2023

Well, the timeout is two seconds. There are legitimate solutions to problems that fail this timeout criteria. Compilation time is the most common case of this. But regex should be fine... we've mostly had issues with syn in the past.

I would be concerned if you consistently received the timeout error message, but the original problem was that there is no message at all? I'm not convinced timeouts / long compile times are the culprit here.

@per-oestergaard
Copy link
Author

I have recorded a video of several attempts with the hello-world exercise and having regex as dependency. I am only adding and removing a space to be able to trigger the test run again. It looks a lot like timeout to me. Note also the last error message I managed to reproduce.
Screencast from 04-11-2023 12:02:38.webm

@senekor
Copy link
Contributor

senekor commented Nov 6, 2023

Thanks again for reporting and investigating. I guess no matter the timeout threshold we choose, there will be some code / dependency list that's right on the edge, so it will sometimes fail and sometimes pass. But hello world + regex seems like a combination that should compile. Also, the non-deterministic error messages is something we need to fix. If there's a timeout, that's fine, but the error message should be clear about it.

@per-oestergaard
Copy link
Author

The timeout is not in the error message. I agree and understand you need a timeout threshold. Besides denial-of-service and unwanted payloads, I would think there is also a processing bill to pay.

I was thinking of whether any caching could be made. E.g. perhaps some of compiling work/state can be kept in an artifact, so the same module does not have to be compiled over and over. See "Shared Cache" in https://doc.rust-lang.org/cargo/guide/build-cache.html.

I managed to get my two exercises through by doing several/many attempts.

@senekor
Copy link
Contributor

senekor commented Nov 15, 2023

I don't know a simple fix for the error message and I can't invest time to debug if I can't reproduce. But I've go the idea in my head to rewrite the test runner, because it's now on v1 of the specification. v3 or even v2 would be nice to achieve. A rewrite might accidentally fix such bugs too (and introduce new ones, obviously).

As for the build cache, yes, I have been working on something like that. I ran into some issues and I currently don't have the time to revisit the topic.

@per-oestergaard
Copy link
Author

@senekor, thank you for the answer. And I understand the resource constraints.

I found the test runner code at https://github.com/exercism/rust-test-runner. I am not sure what you mean by v2/v3 of the specification, perhaps that is outside the test runner and how the test runners is invoked by the larger system.

My understanding of rust-test-runner is that the crates are installed into the runner's docker image (local-registry) and as such, an idea could be to cache the compiled versions locally in the same docker image. Maybe that was what you attempted?

@senekor
Copy link
Contributor

senekor commented Nov 16, 2023

yes and yes.

This issue contains information about the test runner spec.

This draft PR contains my work on the compilation cache. I used sccache, but maybe just a shared target directory could also do the trick and cause less trouble. I don't remember considering this option.

@per-oestergaard
Copy link
Author

I'll await the prune dependencies PR being merged before looking closer. (great work)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants