Skip to content
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

Sync Up Existing Test Suites with x-common #331

Closed
6 tasks done
abo64 opened this issue Jan 31, 2017 · 22 comments
Closed
6 tasks done

Sync Up Existing Test Suites with x-common #331

abo64 opened this issue Jan 31, 2017 · 22 comments
Labels

Comments

@abo64
Copy link
Contributor

abo64 commented Jan 31, 2017

There are continuous changes of existing tests in x-common.
Is there a good way to keep track and update the Scala tests?

Just to list some recent changes:

  • scrabble-score
  • nucleotide-count
  • phone-number
  • prime-factors
  • etl
  • secret-handshake
@abo64 abo64 added the question label Jan 31, 2017
@ErikSchierboom
Copy link
Member

Coincidentally, an issue for this topic has only recently been opened: exercism/problem-specifications#524 Perhaps you could chip in there?

@abo64
Copy link
Contributor Author

abo64 commented Feb 2, 2017

What do you think about the idea to directly parse the canonical-data.json file as suggested in this Haskell issue?

@ricemery
Copy link
Member

ricemery commented Feb 2, 2017 via email

@abo64
Copy link
Contributor Author

abo64 commented Feb 2, 2017

@ricemery Yes, that looks like the right idea.

I wonder whether we should even bother to generate extra Scala source code from the TestCases as it complicates matters quite a bit.
Maybe just use the TestCases to run the tests immediately? (very similar to the Haskell PR).

(btw, you use play.api.libs.json. Why?)

@ricemery
Copy link
Member

ricemery commented Feb 2, 2017

If I understand the Haskell PR correctly, they want their test code to read the JSON and build/execute test cases dynamically.
I'm not sure I like that idea. It add complexity for the end user trying to figure out how to code their solution to meet the test requirements. When I solve exercism problems, I always look at the test code to figure out how to proceed. I suppose this is TDD.
I think having the test cases obfuscated within the JSON makes doing TDD more difficult.

Though I do agree it would make maintaining the test suites easier.

I am using play json because I knew how to use the api. It is in no way a statement on play json being the best way to parse json.

@abo64
Copy link
Contributor Author

abo64 commented Feb 3, 2017

True, this approach works only if a closer look at the test suite is not required (which is the case in the Haskell track). Some issues:

  • Every exercise includes a stub with the signature for the solution;
    or if we don't want this we need a unified format of the tests that makes it very clear how the solution signature must be: For example always having a (possibly overloaded) function named callMUT or so that then calls and reveals the required signature(s).
  • The test failure messages must be self-explanatory including input, expected and actual output.
  • There are no pending exercises (which I personally would welcome - one usually tries to go through the tests from top to bottom anyway to make them green).
  • Maybe some additional general information about all this on how to run a test suite.

I agree that these are a quite disruptive changes, and that might be reason enough to not go this way.
On the other hand the extra step of creating a Scala source file adds quite some complexity and work for ourselves. Is it really worth the effort?

@ErikSchierboom
Copy link
Member

Creating a Scala source is more work, but I really feel we should keep it that way. Every single exercise I try to solve, I start out with examining the tests. I really like having them be explicit and being able to run or debug them individually.

To solve the problem of automatically keeping up-to-date with the canonical data, I think a better approach would be to write a generator that takes the canonical data and automatically creates the test file.

@abo64
Copy link
Contributor Author

abo64 commented Feb 3, 2017

All right, if that is the majority opinion I will of course follow.
I will look upon it as a challenge to simplify this translation process. :)

@ricemery
Copy link
Member

ricemery commented Feb 3, 2017 via email

@abo64
Copy link
Contributor Author

abo64 commented Feb 3, 2017

No, that is fine.
I was thinking to extract some utility functions or so. Your generators are a good start!

@abo64
Copy link
Contributor Author

abo64 commented Mar 6, 2017

Now the x-common JSON schema has become more standardized.

My suggestion is:
We track progress of this issue and update the Scala test suites more or less in sync with it, ideally adding respective test generators.
Even better would be to make use of the schema versions somehow?

What do you think?

@ErikSchierboom
Copy link
Member

So are you suggesting that we should make the basic test generator conform to the updated JSON schema?

@abo64
Copy link
Contributor Author

abo64 commented Mar 6, 2017

@ErikSchierboom Not really, but when I think about it: This is actually a good idea, too! :)
We could do this first. Probably with a clone of TestBuilder (named TestSuiteBuilder, say) in order not to interfere with existing generators.

As this issue is about being in sync with changes in x-common: My initial thought was only to monitor the issue and update our generators and test suites more or less in sync with it.
Or if we somehow keep track of the json file versionIds (for example as a comment in the test suite?) we might even be able to automatically generate the list of outdated test suites?

@rbasso
Copy link

rbasso commented Mar 6, 2017

We are not supposed to change the test data in exercism/problem-specifications#625, except for adding the property key and maybe make other minor changes to pass validation. I'm also taking the opportunity to reformat the files in separated commits.

If any PR there changes the test contents, it is a bug. 🐛

Of course, all the current generators will almost certainly stop functioning.

@abo64
Copy link
Contributor Author

abo64 commented Mar 6, 2017

@rbasso You are right. Maybe I should then say:
Let's use the changes of tests in exercism/problem-specifications#625 to

  • write/adapt Scala test generators for them.
  • cover any changes of outdated Scala test suites on the fly.
  • detect any future changes automatically if we can to keep the Scala test suites up to date.

@ErikSchierboom
Copy link
Member

ErikSchierboom commented Mar 6, 2017

Sounds like a good plan!

@abo64
Copy link
Contributor Author

abo64 commented Mar 6, 2017

TestCaseGen should be modified to contain all possible json schema fields.
And the Json parser can be included in TestSuiteBuilder to always (?!) create the Seq[TestCaseGen].

Then hopefully the default generator will just work for most test suites.

@rbasso
Copy link

rbasso commented Mar 12, 2017

TestCaseGen should be modified to contain all possible json schema fields.
And the Json parser can be included in TestSuiteBuilder to always (?!) create the Seq[TestCaseGen].

Then hopefully the default generator will just work for most test suites.

I just remembered one important thing...

One of the central features of the new schema is that test data is completely separated from test grouping, so it is possible to write generators that are not sensible to test grouping, and they would continue working if the tests are regrouped/"renested", which is not considered a MAJOR change. In fact, if the test ordering is not changed, regrouping is considered simply a PATCH by the proposed semantic versioning.

abo64 added a commit to abo64/xscala that referenced this issue Mar 27, 2017
abo64 added a commit to abo64/xscala that referenced this issue Mar 28, 2017
abo64 added a commit to abo64/xscala that referenced this issue Mar 28, 2017
abo64 added a commit to abo64/xscala that referenced this issue Mar 29, 2017
abo64 added a commit to abo64/xscala that referenced this issue Mar 29, 2017
abo64 added a commit to abo64/xscala that referenced this issue Mar 29, 2017
abo64 added a commit to abo64/xscala that referenced this issue Mar 29, 2017
ricemery added a commit that referenced this issue Mar 31, 2017
ricemery added a commit to ricemery/xscala that referenced this issue Sep 27, 2017
ricemery added a commit to ricemery/xscala that referenced this issue Oct 1, 2017
ricemery added a commit to ricemery/xscala that referenced this issue Oct 2, 2017
ErikSchierboom added a commit that referenced this issue Oct 2, 2017
ricemery added a commit to ricemery/xscala that referenced this issue Oct 2, 2017
ricemery added a commit to ricemery/xscala that referenced this issue Oct 2, 2017
ricemery added a commit to ricemery/xscala that referenced this issue Oct 2, 2017
ricemery added a commit to ricemery/xscala that referenced this issue Oct 2, 2017
ErikSchierboom added a commit that referenced this issue Oct 3, 2017
Update ChangeTest to version 1.1.0. Refs #370, #331
ErikSchierboom added a commit that referenced this issue Oct 3, 2017
Create WordCountTestGenerator. Update WordCountTest. Refs #370, #331
ErikSchierboom added a commit that referenced this issue Oct 3, 2017
Update SumOfMultiplesTest to version 1.1.0. Refs #370, #331
ErikSchierboom added a commit that referenced this issue Oct 3, 2017
Update RnaTranscriptionTest to version 1.0.1. Refs #370, #331
ErikSchierboom added a commit that referenced this issue Oct 3, 2017
Update PigLatinTest to version 1.1.0. Refs #370, #331
ErikSchierboom added a commit that referenced this issue Oct 3, 2017
Update PangramTest to version 1.1.0. Refs #370, #331
ErikSchierboom added a commit that referenced this issue Oct 3, 2017
Update HammingTest to version 2.0.0. Refs #370, #331
ErikSchierboom added a commit that referenced this issue Oct 5, 2017
Create MeetupTestGenerator. Update MeetupTest. Refs #370, #331
@ricemery ricemery closed this as completed Oct 5, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

4 participants