exercism / discussions

For discussing things like future features, roadmap, priorities, and other things that are not directly action-oriented (yet).
37 stars 5 forks source link

Versioning in tests #239

Open iHiD opened 5 years ago

iHiD commented 5 years ago

For the automated mentoring project, it is valuable to know what version of the test data things are being solved against. Tracks are tending to add a comment to the tests file to achieve this, which is a sensible solution in my opinion. However, the formats different signifcantly. For example:

Csharp:

// This file was auto-generated based on version 1.7.0 of the canonical data.

Ruby

# Common test data version: 1.0.0 e1c6d44

PHP

/**
 * @exercism-version 1.4.0
 */

I think it would be nice to standardise on a string format that we use across tracks. Does anyone have any opinions or thoughts on this, or what we should choose?

rossbearman commented 5 years ago

For clarification, the PHP format was just a suggestion, no versioning is currently present in the PHP track repo.

Some form of annotation could make parsing simpler for languages with reflection libraries (or static analysers) that make accessing comment annotations simple without having to manually parse the file contents.

rpottsoh commented 5 years ago

I'm pretty easy going.... I declare a constant, that is currently not being used, just a visual reference for me. When I started doing this at the time I figured it might be easier to access an actual constant rather than having to parse source code, but I would still be able to do either pretty easily; just leaving it wide open for whatever direction my future-self decides to go with it.

  CanonicalVersion = '1.0.0';

is what I do... If I in someway deviate from the test data I will add a fourth element. That seldom happens.

SleeplessByte commented 5 years ago

For tooling, please use semver everywhere. Nothing less nothing more. This is not a comment on where it should be, or how it should be written, just the format.

sshine commented 5 years ago

The Haskell track places a version: MAJOR.MINOR.PATCH.SERIAL in its package.yaml for each exercise, where MAJOR.MINOR.PATCH is the version from canonical data, and SERIAL "must start at 1 and always increase a unit when changed, regardless of the changes in the other version numbers" cf. exercism/haskell#522.

The OCaml track aims to push for something similar, but a (version MAJOR.MINOR.PATCH) in dune-project cf. exercism/ocaml#283. The syntax is Lisp S-expressions because the OCaml compiler uses them extensively. I skipped the SERIAL part because I figured I might be the only one who did it for a long while, and that seemed lonely. The JavaScript track seems to have chosen the same path in exercism/javascript#628, but clearly for less lonely reasons. :-D


@SleeplessByte wrote:

For tooling, please use semver everywhere. Nothing less nothing more.

Forgive my laziness for not investigating, but does semver argue against the Haskell track's additional last SERIAL component? It was added so that we can keep track of track-specific differences that are not reflected in canonical data changes.

A critical property of the SERIAL component is that it is monotonically increasing. We cannot reasonably compare the size of a track-local change to a canonical data change, so we succumb to say that all track-local changes are semantically smaller than canonical data changes.

I think it would be nice to standardise on a string format that we use across tracks. Does anyone have any opinions or thoughts on this, or what we should choose?

Besides @SleeplessByte's comment on semver, I'd like to extend a comment on placing this version:

There is no one common file in an exercise directory for all tracks, neither among those that are distributed to the students or the ones that are not. So I don't think we can easily settle on a standard place. Some languages have package managers where it is natural to place the version number in the exercise's package configuration, which then gets distributed to the student.

So here is a thought:

  1. The canonical version should be a predictable part of the track's exercise version
  2. Place the exercise version anywhere where it can easily be extracted with a program
  3. Extend configlet with a sub-command that extracts canonical versions for tracks that support this

    E.g. configlet version [<path/to/track> [--only <exercise>]]

(There is .solution.json in the student's downloaded directory, and this one also contains "id": "<hash>", which I think is connected to which version of the test data that the CLI should be looking for. But it isn't a git commit hash for the track repo, which I thought!)

SleeplessByte commented 5 years ago

Forgive my laziness for not investigating, but does semver argue against the Haskell track's additional last SERIAL component? It was added so that we can keep track of track-specific differences that are not reflected in canonical data changes.

It doesn't. It only defines the first three components.

Additional labels for pre-release and build metadata are available as extensions to the MAJOR.MINOR.PATCH format.

The SERIAL is considered an extension and perfectly valid.

NobbZ commented 5 years ago

Personally I'd like to see versions as I'm used to them from my package manager…

They just use upstream versions, and if they have to change the build by aplying patches or because a dependency got updated which requires recompilation, then they increase the release version. Whenever the upstream version gets bumped, the release version gets reset to 1.

As an example:

Upstream version of hello-world is 1.0.0, in this example it works in the old way, like two-fer does today.

Erlang implements it using regular erlang strings and only accepts those as an answer. Its 1.0.0-1. Erlang changes this exercise to accept "new" strings which are some trickery mixed and nested stuff of lists and arrays of integers. A module is there to wrap them, and which is also able to recognize "foo" and [$f, <<"o">>, "o"] as equal. Its 1.0.0-2. Upstream changes hello-world to be what we know today as 2.0.0, just return a string greeting the world… Erlang implements this straight using the strings module and accepts any return value that is treated equal to <<"Hello, World!">> by this module. Its version is 2.0.0-1 then.

wolf99 commented 5 years ago

It might be wise to bear in mind that versioning may also be used for things other than automated mentoring . For example, automated canonical version change notification.

If we do not want to end up with different version values in different places within the same track to accommodate different automation flows, then a common location might be wise - perhaps config.json?

NobbZ commented 5 years ago

For tooling, please use semver everywhere. Nothing less nothing more. This is not a comment on where it should be, or how it should be written, just the format.

We are not talking about tooling, we are talking about the version of the tests, where we have to deal with the fact, that the canonical data might have a version on which the testsuite of a track is based on, but without the canonical data changes the testsuite might evolve. See my hello-world example from above.

If we do not want to end up with different version values in different places within the same track

Yes, I second this, try not to duplicate this version over all over the repository.

perhaps config.json?

Don't, please… There is already too much in the config.json, and especially I do not want my test generator having to update that one as well…

I'd prefer if we had a file in exercises/$exercise/.meta which we could use.

As far as I remember @exercism/haskell already uses that location.

Still, I probably had to duplicate the version number sooner or later, as an ultimate goal of mine is, to have the version of the testsuite in the rebar.config (project description, easy said). But once I am close to that point, I could easily add a test to travis to ensure that the version in rebar.lock and .meta/version (or whatever that file will be called) are insync.

petertseng commented 5 years ago

In this comment I purely address a previous comment that was made, without adding any additional thoughts of my own.

I'd prefer if we had a file in exercises/$exercise/.meta which we could use. As far as I remember @exercism/haskell already uses that location.

I have knowledge that the Haskell track does not place such a version file in exercises/$exercise/.meta. As evidence, I submit the following output from the following command:

[pt@ulysses:exercism/haskell(master u=)]$ ls -1 exercises/*/.meta/* | cut -d/ -f4 | sort | uniq -c
      1 DONT-TEST-STUB
     42 hints.md

Neither of these files contain any version information.

petertseng commented 5 years ago

Looks like the original question was asking only about a string format, and was not really discussing where any such version indicator should exist. So I will save my comments on location for a different day and maybe a different issue. But as a spoiler, I will share now that I do not see any clear benefit in requesting (or forcing) tracks to adhere to a standard location.

Regarding the format, I wonder if the most naive solution would work: Any string of the form "x.y.z" will be treated as the version. It is an error if more than one such string is present in the file. Actually, this doesn't quite work because phone-number has in its canonical data the test input "223.456.7890" which will be detected as the version. I would suggest being even more naive and say "if there's more than one, just take the first one", but I think silently having this sort of behaviour is too prone to misjudgments.

However, maybe instead we can take this strategy:

Look for a line that contains the word version. It is an error if there is more than one. From that line, look for one string of the form x.y.z. That is the version.

Since this strategy (well, okay, it looks for "Problem Specifications Version") works for the entire Go track according to https://github.com/petertseng/exercism-problem-specifications/blob/up-to-date/up-to-date/deprecated-go.rb, I feel no qualms at all for suggesting it. Maybe it's less work than asking individual tracks to conform to a standard that might not even make sense for their track.

ErikSchierboom commented 5 years ago

For the automated mentoring project, it is valuable to know what version of the test data things are being solved against.

Is this valuable as an analyzer could then use different paths to analyze different versions? Or perhaps skip older versions or something like that?

NobbZ commented 5 years ago

Is this valuable as an analyzer could then use different paths to analyze different versions?

Probably… Currently I have a rule in the erlang analyzer that applies to all exercises.

If there is a function with the name test_version of arity 0 exported from the module under test, then "warn" the student to update the testsuite.

Well, thats the plan once I go online.

If though I knew the exact testversion as I used it internally in the track (given I had used them back in the days). I could warn to update on old versions while making it a hard error on newer versions of the exercise.

bencoman commented 5 years ago

While I can't identify a particular case, my gut feel is that version on own is susceptible to a name clash. I like PHP's exercism-version: but I'd go further and reference the source file canonical-data-version: or canonical-version:.

ErikSchierboom commented 5 years ago

If we were to use a simple string identifier, my preference would also be canonical-data-version: X.Y.Z

NobbZ commented 5 years ago

As I said earlier, I consider it important to be able to keep track of track local evolutions of the test suite against the same canonical test data.

Be it because of language updates, fixing bugs in the testsuite, etc.

Therefore, as currently the canocical data has 3 digits, we need at least 4 in the tracks. Where tracks should be able to decide on their own if they monotonically increase this version as haskell does, or if they reset on canonical updates as I did in my example (and will introduce in erlang soonish).

I like the - to delimit canonical version and local version, because it clearly visualizes even for the human, that the following part is different and does not belong to the canonical version. Also it is easily splitable into a regular version string which most languages have parsers for and the local number. If we use a . as currently done in haskell it slightly increases preparation complexity.

SleeplessByte commented 5 years ago

Therefore, as currently the canocical data has 3 digits, we need at least 4 in the tracks. Where tracks should be able to decide on their own if they monotonically increase this version as haskell does, or if they reset on canonical updates as I did in my example (and will introduce in erlang soonish).

This is exactly why @sshine has that in the Haskell versioning.

Is this valuable as an analyzer could then use different paths to analyze different versions?

Yes. I can actually then have analyzers that:

iHiD commented 5 years ago

A reminder that the aim here is that the files that get to the student or get to the analyzer have the version. We couldn't therefore put it in config.json. The options are a new file that has to be in all exercises, in the code (presumably via a stub file) or via the test file.

There seem to be a few different things being discussed, so to move this forward and add some clarity:

1) Is anyone objecting to putting a SEMVAR version in the test files? 2) Is anyone objecting to putting it as a comment that follows the same format across all tracks (with the acknowledgment that the prefix to the comment might be different per language - e.g. //, /*, # etc)?

If you are objecting to (1) or (2) could you please explain why (referencing the number). Thanks :)

NobbZ commented 5 years ago

A reminder that the aim here is that the files that get to the student or get to the analyzer have the version.

Thats bad… I'd hoped it were somewhere written into the metadata…

From all what I have read so far, all I will see in the analyzer, is the submitted files. So the only place I could put a comment were in the stub.

But what if the student doesn't understand the value of that comment and deletes it? What if the student in general deletes the stub and implements from file not found errors alone?

  1. Is anyone objecting to putting a SEMVAR version in the test files?

I have nothing against semver, as long as we as a track are allowed to add tags to track internal revision. I'm not fixed on the - from my examples above, but I do not know semver good enough to actually know hot to encode the revision in a way, that an increased revision also means a greater version.

  1. Is anyone objecting to putting it as a comment that follows the same format across all tracks (with the acknowledgment that the prefix to the comment might be different per language - e.g. //, /*, # etc)?

Totally depends on the where.

Stub? Bad idea, look above…

Tests? Named differently for each exercise, therefore additional tooling as the already mentioned canonical-data-has-updated-notification-bot had to search for it. Then we can as well write individual parsers for each track…

Centralized Meta File per language? Sure, I'm all in! But why a comment then? Just make it a file only containing the version and a newline. But this file won't be available to students or analyzers…

SleeplessByte commented 5 years ago

I have nothing against semver, as long as we as a track are allowed to add tags to track internal revision. I'm not fixed on the - from my examples above, but I do not know semver good enough to actually know hot to encode the revision in a way, that an increased revision also means a greater version.

To reiterate 😄 semver has extensions to encode build numbers, or monotonically increasing numbers, beta version, alpha releases etc.

I believe under most extensions:

'1.2.3-1' > '1.2.2-5'
'1.2.3-1' < '1.2.3-5'
'1.3.0-1' > '1.2.2-5'
'2.0.0-1' > '1.2.2-5'
SleeplessByte commented 5 years ago

Centralized Meta File per language? Sure, I'm all in! But why a comment then? Just make it a file only containing the version and a newline. But this file won't be available to students or analyzers…

I think I prefer this. Why doesn't the student or analyzer have this? I think there is a discussion to give analyzers the full directory as per the git commit for THAT submission?

petertseng commented 5 years ago
  1. Is anyone objecting to putting a SEMVAR version in the test files?

I originally considered declaring this suboptimal but acceptable but after further consideration have decided to withdraw that judgment, replacing it with no objection. Below are:

  1. original reason why I was about to say this is suboptimal
  2. why I decided to withdraw it

suboptimal?

I considered languages with commonly used tools that can place a version in a location known to the tool. examples:

So what would happen if version goes in a comment in the test file? One of two things.

  1. those language tracks must duplicate the version. once in the place where the language tool expects it, once in the tests
  2. those language tracks choose not to duplicate the version. instead they only place it in the tests and give up on placing the version in the location known to the tool.

I considered both possibilities suboptimal.

further consideration

Actually, I thought about it and don't really see a benefit to letting the language tool (Cargo, Stack, etc.) know about the version. Unless someone convinces me there is a reason I withdraw the objection.

SleeplessByte commented 5 years ago

Actually, I thought about it and don't really see a benefit to letting the language tool (Cargo, Stack, etc.) know about the version. Unless someone convinces me there is a reason I withdraw the objection.

Right. In JS/TS, we wanted to use the package.json version, but for our CI, it actually doesn't matter if it lives in a different file whatsoever. It also doesn't matter in terms of maintaining.

rpottsoh commented 5 years ago

I've just finished reviewing the conversation above and I am left wondering if the CLI should play a role here. If we are thinking version should be in the solution but realizing the student could remove it, the CLI could be used to make sure that student submissions contain version. The CLI could possibly insert it, I am making the assumption that the CLI could determine what the correct version for that student. The correct version for the student may not be the latest version of the exercise, if the student hasn't upgraded to the latest version of the exercise.

SleeplessByte commented 5 years ago

I am making the assumption that the CLI could determine what the correct version for that student

The correct version is the version their solution is git-tagged with. I don't want the student to be able to submit or change the version at all.

The correct version for the student may not be the latest version of the exercise, if the student hasn't upgraded to the latest version of the exercise.

Yep, that's why exercise-downloads are tagged with a sha in the first place in order to be able to show that button. We can re-purpose that to get the correct version file.

NobbZ commented 5 years ago

The correct version is the version their solution is git-tagged with.

Which in no way can be resolved to the version of the problem spec the exercise is based on.

Also for an anylyzer it is very hard to decide whether or not the SHA 1337beaf is newer or older than b347leaf…

I don't want the student to be able to submit or change the version at all.

Yes, whatever the student does, it should not change the version my analyzer sees.

SleeplessByte commented 5 years ago

Which in no way can be resolved to the version of the problem spec the exercise is based on.

Correct. This is why I wanted versions in the first place. The only way to map this is to hold a curated mapping, which is error-prone and honestly not a good solution.

sshine commented 5 years ago

@iHiD:

[...] to move this forward and add some clarity:

  1. Is anyone objecting to putting a SEMVAR version in the test files?
  2. Is anyone objecting to putting it as a comment that follows the same format across all tracks (with the acknowledgment that the prefix to the comment might be different per language - e.g. //, /*, # etc)?

If you are objecting to (1) or (2) could you please explain why (referencing the number). Thanks :)

Two objections:

@NobbZ:

1. what if the student doesn't understand the value of that comment and deletes it?

  1. This complicates extraction compared to a .meta/version file with exactly the canonical version in it and nothing else.

If we find this version in much the same way as the exercism CLI tool finds the right test file when downloading, the user cannot mangle it. The only downside is that .meta/version won't be able to hold a 4th component to the version which some tracks use for updates to test files that don't reflect canonical test updates.

Would a satisfying compromise to use .meta/version and to have any additional versioning information be in sync with this? This way Haskell track can keep the version: x.y.z.s format in its package.yaml, Tcl track can keep a set version x.y.z.s in its test.tcl, etc. along with what analysers, test versioning reminder tools and whatnot need from .meta/version, and CI hooks can ensure that they're in sync.

SleeplessByte commented 5 years ago

The only downside is that .meta/version won't be able to hold a 4th component to the version which some tracks use for updates to test files that don't reflect canonical test updates.

It would. 4 components is fine in semver! It's the "build-extension" IIRC.

Other than that, I'd love to have it in the cli tool as well!

ErikSchierboom commented 5 years ago

It would. 4 components is fine in semver! It's the "build-extension" IIRC.

Yep. One can use a dash to mark pre-release versions (e.g. 1.0.0-alpha), but in think case I think it would make more sense to use the plus sign to mark it as metadata (e.g. 1.0.0+20).

I'm in favor of having the version in .meta/version, I think moving it into a file that can be edited by the student is more likely to lead to errors.