Skip to content

Clock: Add deriving Eq to stub to make it testable #891

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

Merged
merged 1 commit into from
Jan 21, 2020
Merged

Clock: Add deriving Eq to stub to make it testable #891

merged 1 commit into from
Jan 21, 2020

Conversation

sshine
Copy link
Contributor

@sshine sshine commented Jan 20, 2020

Clock is the last remaining exercise that has the .meta/DONT-TEST-STUB file because the type in the src/Clock.hs stub is not compatible with the test suite.

Other examples of exercises that were revised so that testing the stub became possible are listed in #466. The difference is that those all had data definitions, whereas Clock is ideally solved with a 'newtype' and an appropriate alias.

The DONT-TEST-STUB mechanism was added by @petertseng in #464 and in particular the commits

The change made in this commit argues that since the example solution has deriving Eq, adding this to the stub will make the exercise a bit easier, but it still won't give away the central piece.

The motivation behind removing DONT-TEST-STUB in Clock is that with this single effort, we can say that all stubs can be compiled and tested. To motivate further, I will cite @rbasso in #466:

I think that this is the way to go. We have to sacrifice a little
flexibility in designing exercises, but we gain a more coherent
user-experience and automatic testing of stubs. 👍

This bumps the exercise's version from 2.4.0.9 to 2.4.0.10.

Clock is the last remaining exercise that has the .meta/DONT-TEST-STUB
file because the type in the src/Clock.hs stub is not compatible with
the test suite.

Other examples of exercises that were revised so that testing the stub
became possible are listed in #466. The difference is that those all had
`data` definitions, whereas Clock is ideally solved with a 'newtype' and
an appropriate alias.

The DONT-TEST-STUB mechanism was added by @petertseng in #464 and in
particular the commits

 - 1d91ceb (Travis code)
 - 342b932 (Clock-specific)

The change made in this commit argues that since the example solution
has `deriving Eq`, adding this to the stub *will* make the exercise a
bit easier, but it still won't give away the central piece.

The motivation behind removing DONT-TEST-STUB in Clock is that with this
single effort, we can say that all stubs can be compiled and tested. To
motivate further, I will cite @rbasso in #466:

> I think that this is the way to go. We have to sacrifice a little
> flexibility in designing exercises, but we gain a more coherent
> user-experience and automatic testing of stubs. 👍

This bumps the exercise's version from 2.4.0.9 to 2.4.0.10.
@sshine
Copy link
Contributor Author

sshine commented Jan 20, 2020

Additionally, I have two notes for reference:

  • We can still, for now, keep the functionality in .travis.yml.
  • In the context of adding "concept exercises" for "Exercism v3", we can simplify the union of the current track repo's directory structure and what is proposed as the future directory structure once "concept exercises" become a thing. (In regards to the many consequences that that pull request would have on this repo, I have remarked there that I intend to create issues in this repo's issue tracker discussing each point.)

Copy link
Member

@petertseng petertseng left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

wow, we must have completely forgotten to do this when doing #695 . The DONT-TEST-STUB file mentions the Num instance, but #695 has told us that the Num instance is a bad idea. Adding deriving Eq to the stub to make it testable is eminently reasonable and should have been done a long time ago, so this PR is just doing what we forgot to do back then.

@sshine
Copy link
Contributor Author

sshine commented Jan 21, 2020

@iHiD: I'm a bit unsure why this happens:

wat

Could it be related to my recent permission changes in relation to the exercism/v3 repository?

@petertseng
Copy link
Member

I would like to note for the record that I am also not authorised, but that I assume this problem will be solved for the entire Haskell maintainer team at once, so perhaps I need not have mentioned it.

@iHiD
Copy link
Member

iHiD commented Jan 21, 2020

Should be fixed for this repo now. It's currently broken everywhere.

@sshine sshine merged commit a7eed6e into exercism:master Jan 21, 2020
@sshine sshine deleted the clock-test-stub branch January 21, 2020 12:23
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

Successfully merging this pull request may close these issues.

3 participants