exercism / haskell-test-runner

GNU Affero General Public License v3.0
1 stars 13 forks source link

Update to resolver 20.11 #72

Closed mx-ws closed 1 year ago

mx-ws commented 1 year ago

Hi, in the context of this pull request I also updated the resolver to 20.11 here.

In the process I changed the expected results of the tests. Notice how I changed the run-tests.sh script to work on my macOS machine.

I'm not at home and therefore do not have access to my Linux machine. Therefore feedback whether the tests also succeed on Linux would be appreciated.

github-actions[bot] commented 1 year ago

Hello. Thanks for opening a PR 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 PR has been pre-approved, please link back to this PR on the forum thread and a maintainer or staff member will reopen it.

mx-ws commented 1 year ago

Hi, excuse my inactivity, I'm a bit busy at the moment.

The errors are associated with the fact that the sed command apparently leaves the absolute path to the project in the output (/Users/mxw/... in my case).

Nice work! Would the old solutions work with the new resolver?

I'm not sure if I understand. Which old solutions do you mean exactly?

ErikSchierboom commented 1 year ago

I'm not sure if I understand. Which old solutions do you mean exactly?

Solutions that have already been submitted by students.

mx-ws commented 1 year ago

Solutions that have already been submitted by students.

I can tell you about my three or so solutions that still worked, but otherwise I don't feel confident in answering this question as I'm not an expert about the ghc. As a reminder, the version step here is 9.0.2 -> 9.2.5 of ghc or 19.27 -> 20.11 of resolver.

hesselink commented 1 year ago

@mx-ws As you suggested in https://github.com/exercism/haskell/pull/1158#issuecomment-1519150246, I've made a few updates to your PR: https://github.com/exercism/haskell-test-runner/compare/main...hesselink:haskell-test-runner:lts-20. This passes all tests, and it also updates the docker tag to GHC 9.2 (which also ensures there's an M1 image) and sets the lts to the latest (20.18, using GHC 9.2.7). This is also the 'recommended' version in ghcup.

mx-ws commented 1 year ago

Thanks for helping out! Notice how we already had a discussion at https://github.com/exercism/haskell/pull/1158 about to which resolver to upgrade. You are right in that ghcup's recommendation has switched from 9.2.5 to 9.2.7 since then. I'm pretty neutral about this, I just think we should use the same version in both repositories.

I guess if no one intervenes or comments otherwise I'm going to set resolver 20.18 over at https://github.com/exercism/haskell/pull/1158 and pull your changes here at this repository.

hesselink commented 1 year ago

Yeah, I saw that discussion, but since the consideration was what ghcup recommends, I figured switching to the latest point release was best (include the most fixes, without much risk of breaking). Your path forward sounds great, thanks!

ErikSchierboom commented 1 year ago

I'm not sure if the sed thing works, the CI run says:

"sed: can't read : No such file or directory"

for each test

hesselink commented 1 year ago

That's weird, I'll have another look. I tested it locally on mac and in docker, and both seemed to work.

hesselink commented 1 year ago

I've pushed a fix to my branch. Turns out I accidentally committed the mac fix to the sed command in a later, unrelated commit. I've removed it now. @mx-ws Why did you decide to remove your code to have it work on both mac and linux?

hesselink commented 1 year ago

@ErikSchierboom We could consider failing the scripts on any errors. Something like this: https://github.com/exercism/haskell-test-runner/commit/4928c04a48e1c24416d5cae456060d1556fc74c1. If you're open to this, I can make a proper PR.

ErikSchierboom commented 1 year ago

If you're open to this, I can make a proper PR.

Yes please!

hesselink commented 1 year ago

Great! I created #77

ErikSchierboom commented 1 year ago

Great, merged that PR. Could you rebase?

mx-ws commented 1 year ago

I've pushed a fix to my branch. Turns out I accidentally committed the mac fix to the sed command in a later, unrelated commit. I've removed it now. @mx-ws Why did you decide to remove your code to have it work on both mac and linux?

  1. My understanding is that the haskell-test-runner project will not run on a user's machine (and therefore it's not really an issue if it can't run on M1 in the same way as the haskell repository).
  2. I could not seem to get the run-tests.sh script working for both mac and linux at the same time.

So I chose to rollback the corresponding changes and stop putting time into this particular issue. For running the tests locally I just started using docker.

ErikSchierboom commented 1 year ago

CI seems to be failing

hesselink commented 1 year ago

It looks like it's missing my fix from https://github.com/exercism/haskell-test-runner/pull/72#issuecomment-1521796834. @mx-ws Could you cherry-pick the latest commit from https://github.com/exercism/haskell-test-runner/compare/main...hesselink:haskell-test-runner:lts-20 as well?

On the positive side, the 'fail on error' is working 😄

ErikSchierboom commented 1 year ago

Thanks @mx-ws!