-
-
Notifications
You must be signed in to change notification settings - Fork 22
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
Reverted fetch-cobolcheck and fetch-cobolcheck.ps1 scripts. #158
Reverted fetch-cobolcheck and fetch-cobolcheck.ps1 scripts. #158
Conversation
This PR touches files which potentially affect the outcome of the tests of an exercise. This will cause all students' solutions to affected exercises to be re-tested. If this PR does not affect the result of the test (or, for example, adds an edge case that is not worth rerunning all tests for), please add the following to the merge-commit message which will stops student's tests from re-running. Please copy-paste to avoid typos.
For more information, refer to the documentation. If you are unsure whether to add the message or not, please ping |
what say @ErikSchierboom ? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you explain why these changes would fix things? It looks to me like they no longer use a shared directory and thus would re-download cobolcheck for each exercise.
It required root permissions to download cobolcheck to |
Yes, either that or store it in And then update the |
Per the old fetch-cobolcheck source, it fetches it to each repo's ./bin. This PR basically reverts back to the old functionality by adding
|
It makes sense. We can try to fetch cobolcheck and download it to |
Using the repo root is fine too. I don't really care as long as one place is used |
- Renamed bin/sync-fetch-cobolcheck-files to bin/sync-exercise-files and added synchronization for test.sh and test.ps1 files. - Added bin/exercise_test.sh and bin/exercise_test.ps1 as templates for test.sh and test.ps1 respectively. - Changed cobolcheck location to $HOME/cobolcheck/cobolcheck{,exe}.
@kapitaali @ErikSchierboom Now cobolcheck is stored in |
…check/ creation in the fetch-cobolcheck,ps1
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's try this out
@kapitaali @ErikSchierboom could we merge it? |
Done! |
The tests should work out of the box if you download the exercise with cli. I deleted the anagram directory, downloaded it with cli, copied my source code to src, ran test.sh and got the following:
I'm in favor of reverting back to the old functionality without any modifications where test.sh downloads the cobolcheck binary to the local ./bin directory in the exercise tree. |
@kapitaali Did you try |
It should make no difference, but trying ./test.sh gives me
|
I got |
Doing chmod +x test.sh allows me to execute it by running ./test.sh, but the errors persist:
|
Exercism COBOL documentation says that you have to run |
Hmm.. It's weird. It works for me. What's your OS? |
This is Pop!_OS, an Ubuntu variant. I am just wondering why would we need to touch Linux shell scripts at all if the original intention was to do something about Powershell. |
We changed |
We now have 1 datapoint that works and 1 that doesn't. Need more. I have a clean debian install, going to try with that. Just a sec. |
Fails on debian too:
|
I believe it works with your system, but running ./test.sh should work everywhere and be configuration independent. Not a single user should have to make any configurations to their system to run test.sh and COBOL tests succesfully. That was how the testing script worked a while ago. |
Could you share your |
Yeah it's here:
|
This is an old version of |
@ErikSchierboom sure. It is weird. It works locally but it can't resolve api.github.com. |
@kapitaali @ErikSchierboom Could it be connected with DNS resolving on the server? And why? |
EDIT: there was nothing wrong apparently? testing works now |
By the errors, it seems that curl try to resolve api.github.com but it can't for some reason. |
I deleted cobolcheck and ran test.sh and fetch-cobolcheck worked fine here too. So it's the test environment that is failing. |
I ran tests on the website and they're working. So it must have been a temporary hiccup that is now fixed. |
Hmm.. interesting |
The exercise that I tried that worked was anagram, the one that was added last. But updating the atbash-cipher exercise on exercism.org gave me the error that was reported:
I'm going to try downloading an updated exercise with the cli to see if this happens locally. |
Running locally the updated exercise that has been downloaded with cli works fine. cobolcheck gets downloaded and tests are run. |
The test runner can't download anything as it does not have internet. Cobolcheck thus must be downloaded to the right location in the test runner dockerfile. Can you check that? |
I checked that locally earlier and it works fine. I ran test runner locally in container and without it. |
What command did you use? |
There are things that have been paused in the workflow: https://github.com/exercism/cobol/actions/runs/10566009241/workflow |
As I remember I used |
WHICH_COBOLCHECK=$(which cobolcheck) | ||
if [[ $? -eq 0 ]] ; then | ||
echo "Found cobolcheck, using $COBOLCHECK" | ||
COBOLCHECK=$WHICH_COBOLCHECK | ||
elif [ ! -f $SCRIPT_DIR/bin/cobolcheck ]; then |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@SimaDovakin I think the remove of this part is what breaks the test runner. The cobol test runner downloads the cobolcheck file to: https://github.com/exercism/cobol-test-runner/blob/main/Dockerfile#L16
What I would suggest to do is:
- Either revert the WHICH part that was deleted, or
- Change the location of cobolcheck in the test runner (and also update the
test.sh
files in thetests/*
directories to match what is used here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm... Ok, I'll return "WHICH" part back.
@ErikSchierboom @kapitaali I reverted WHICH part here #162. But I'm still wondering why the test runner can't resolve |
Attempt to fix the problem with cobolcheck fetching.