exercism / sml

Exercism exercises in Standard ML.
https://exercism.org/tracks/sml
MIT License
27 stars 35 forks source link

Consider using sml-pkg for our test-framework #134

Open sshine opened 4 years ago

sshine commented 4 years ago

This was recently released: https://github.com/diku-dk/smlpkg

We might distribute our test framework, testlib.sml, using this package manager.

@mcmillhj, @snahor: Do you by any chance know the origin of testlib.sml? The commit history doesn't seem to go far enough back to tell me who has the copyright. It seems to have been either designed for Exercism or copied from some courseware.

mcmillhj commented 4 years ago

Pretty sure that was something @snahor made from scratch, I had implemented an earlier version that was much less functional and it was replaced with this version.

snahor commented 4 years ago

Yep, I created it from scratch. After that I created a repo for it https://github.com/snahor/sml-test.

Hans Roman

Hans Roman

On Wed, 22 Apr 2020 at 07:59, Hunter McMillen notifications@github.com wrote:

Pretty sure that was something @snahor made from scratch, I had implemented an earlier version that was much less functional and it was replaced with this version.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or unsubscribe.

rainij commented 2 years ago

I would like to add one observation to the discussion. While working on this https://github.com/exercism/sml/pull/199 PR I observed that when a test fails with an "unexpected" exception, then the test-script does not exit with a non-zero exit code. Hence it looks like all tests run fine although this isn't true.

In that particular case, the test for the exercise "binary" imported example.sml (wrong) instead of binary.sml (correct). Hence it could not resolve the decimal function and failed with an error.

Something similar happens with the "space-age" exercise. My local installation of PolyML seems to be slightly different then the one used by the ci. It uses 31 bits for integers. Hence an overflow is raised when I execute the tests for that exercise (locally), since the numbers are that large. But the exit code is still 0. If this would happen in the ci it would still be wrongly considered as "successful".

It would be nice if the new test-framework could handle such situations correctly.

rainij commented 2 years ago

@sshine @snahor @mcmillhj is anybody of you still involved in this repo? I would like to know if somebody has experience with smlpkg and has a good feeling about integrating it here. Are there alternatives?

sshine commented 2 years ago

I’m not involved, but I can give an assessment.

smlpkg depends in the MLB format which is used by SML/NJ, MLton, MLKit, and SMLtoJs.

Since this track uses Poly/ML, which does not support the MLB format, you would either need to replace the compiler/repl to one that does, or add support for the MLB format in Poly/ML.

Of the mentioned alternatives, SML/NJ is hard to install and use, and MLton is an optimising compiler. MLKit could work (I haven’t tried it), but the least effort would nonetheless be to either add support for a module format, or replace the compiler on there whole track. One of the benefits of Poly/ML is that it’s easy to install (is it still?) and that its error messages are not bad.

If SML were a language used in industry, and the best practice was to use smlpkg, it’d be optimal to learn about this. Or if the availability of packages somehow extends the capability with e.g. fundamental data types and algorithms not found in SML Basis, making some problems easier to solve in an idiomatic way.

I am not to say that’s not the case. But they implementer of an SML package manager on this track would have to assess whether adding a package manager to the mix would teach more of what’s worth learning about SML, or add an overhead.

Ideally, downloading the entire development environment should be one command on popular systems, and assessing whether a solution is correct should be a single command. I’d probably seek to achieve this user experience in any case. I’m not sure how much it is the case currently.

For completeness, it should be mentioned that before smlpkg there was Smackage.

I hope these comments were somewhat coherent.

rainij commented 2 years ago

@sshine thanks for your very detailed feedback! I will keep these things in mind.

guygastineau commented 2 years ago

@rainij I am interested in being more involved here. I am currently doing some necromancy in the scheme track. Between that and work my availability could be spotty though. In the past I modified the test suite for portability across SML implementations, but the test generator would need modifications. I don't want to edit python while working on the SML track, so I looked into rewriting the test generator in SML, but I got sidetracked. I am still interested in that, and I think we can (and should?) make this track work for multiple implementations. That is off topic for this issue.

Anyway, are you currently a maintainer of this repo?

rainij commented 2 years ago

@guygastineau no I am no maintainer but I am interested in being involved here too. I also opened some PRs in the past (mainly due to the fact that the repo was getting not working anymore).

Concerning the test generator I opened an issue for improving/correcting the test generator (see https://github.com/exercism/sml/issues/204). Initially I just wanted to re-implement it in Python again. But our discussion actually went into the direction of implementing it in SML exactly as you suggested it. And indeed I was planing to do it (probably in a few weeks/month).

But if you are interested in that too, maybe it would be a good idea to do it together (in some way). At least one can notify the other one if one starts working on it and ask for a review and discuss things ... .

guygastineau commented 2 years ago

@rainij cool. I have a local branch where I pull in the best JSON library I could find for SML as a git submodule. I'll see if I can get some minimal-like reproduction of the current generator. I'll stop talking about it from here for now. After a little bit of real work is done on it I will make a WIP PR, so you can check out my work.

guygastineau commented 2 years ago

Concerning sml-pkg though...

It looks good. The author of the best JSON parsing lib for SML that I have found is the author for sml-pkg too I believe. I don't think it will work with Poly/ML out of the box though. Honestly, I haven't been doing much SML in the past 10 months, but I migrated from poly to smlnj and mlton in my personal projects. My last several projects were all set up, so I could use smlnj for repl and Milton for release. I thus have a preference for mlton, but I recognize setting things up for smlnj and mlton at the same time is a bit of advanced dot file/build system hackery.

Anyway, for my current effort to rewrite the generator I am using git submodules for dependencies, but that could all be replaced with sml-pkg pretty easily.