nucleotide-count: refactoring tests (discussion) #895

Vankog · 2017-09-08T18:07:11Z

Hi,

In the same vain as the other PR #893 here is the discussion about nucleotide-count.

Background:

I'm currently contributing some bits and tits to the JS and ecmascript tracks and while doing so I came across some issues I see with some of the test files. Upon proposing some changes the maintainers asked me to contribute to the canonical track first, before it can be addressed in the particular track. So here I am. :-)

In this PR I'd like to discuss the test spec for the pangram exercise in particular.

First I have some assumptions:

This is a learning platform, so the 'course material' should be a good example of good practices and (arguably) well designed principles.
As far as I can tell we utilize TDD with a progression in the tests that evolve the desired implementation one test case at a time. This is called "Fake it till you make it" or "sliming" and is a nice practice to focus on what is essential. (see https://www.youtube.com/watch?v=PhiXo5CWjYU for a short vlog about this concept)
Because of this, extra special care should be taken in reviewing contributed test cases. Especially, because they are the main reference material for (mostly) inexperienced learners.

Analysis:

The nucleotide-count test spec is another test that I took a closer look at and at least to me there seem to be some things that could be improved.
Here is the current one:
https://github.com/exercism/problem-specifications/blob/fba1aef6b2237f504bfdd0bf575fd49489f12523/exercises/nucleotide-count/canonical-data.json

The following things came to me:

Some important edge cases are missing. e.g. The null-case or lowercase-cases. Arguably we should even test for wrong input types like objects, arrays etc. in some languages, which is track specific I guess.
The order of the cases seems to be nice enough for a "sliming"-progression, but could be improved with some steps inbetween.

Suggestion:

I am far from an expert in this field. To be fair I'm even a learner who needs practise in TDD. However, I am particularly interested in a good TDD design and the clean code movement. And I think I can add some value from experienced talks and tutorials and videos about testable code and clean code in general to this discussion.

Therefore, I have a suggested version of the nucleotide-count test spec you can see in this PR.
Here is the file in a whole:
https://github.com/Vankog/problem-specifications/blob/refactor-nucleotide-count-test/exercises/nucleotide-count/canonical-data.json

I think I provided a good sliming progression, adding one aspect after another. This mandated some intermediate test cases, like testing a single char input before testing a sequence.
Also I tried to think about the edge cases more carefully and what needs to be tested, adding some and improving the descriptions.

What do you think?

This attempts to improve the BDD/TDD progression ("sliming"), better descriptions, added test cases. exercism#895

Vankog · 2017-09-08T18:11:53Z

exercises/nucleotide-count/canonical-data.json

+              "description": "checks if DNA input does not include invalid characters.",
+              "property": "nucleotideCounts",
+              "strand": "AGUUÄ1+ :'A'}CT",
+              "expected": {


check for some (ASCII compliant) invalid chars.

Vankog · 2017-09-08T18:13:12Z

exercises/nucleotide-count/canonical-data.json

+          "description": "counts an undefined DNA strand as empty strand.",
+          "property": "nucleotideCounts",
+          "strand": null,
+          "expected": {


do i have to write null or "null"?

I don't think null input handling is appropriate to test here.
The point of this exercise is string parsing, not having a string at all defeats the whole purpose of the exercise.

To answer the question you asked: null

I see what you mean.
But as far as I experienced it, test suits should check for edge cases. null checks are especially important (at least in almost all languages).
In every educational piece of material the advice is not to only check for usual inputs, but for all possible (equivalent kinds of) inputs. null checks, empty checks and others are always on top of the example lists given. Just take any arbitrary course about QA and this will be part of it.

I think this is especially important, because we are in an educational context. The users should learn from the getgo what it means to write (arguably good) tests (first), because it is still a major problem, even upon seasoned programmers.
From my own business experience I know that new programmers are completely lost when it comes to writing tests. But also experienced programmers fall in the traps of "I'll do it later", "This is a trivial method", "I can't test this", "There are too many dependencies" etc. Just take any talk or tutorial about writing tests or even TDD and you will see the same old counter-arguments or questions again and again from the comments or the audience. This has a reason: Writing good tests is hard! Very hard to be exact and it is a skill that needs exercise. And where better to begin than right from the start?

I think this concept deserves wider discussion so I've replied as a new issue: #902

Vankog · 2017-09-08T18:13:42Z

exercises/nucleotide-count/canonical-data.json

+            "C": 0,
+            "G": 1,
+            "T": 0
+          }


even tough this case might already be included in the next case, this test is for a nice sliming progression. Developers should have quick success by implementing a subset of the spec first.

Why is returning a hardcoded 1 different from returning a hardcoded 7?

This is a good point.

I agree that this is not necessary. However, why should anyone hardcode against two or more different test cases? Programming means using your brain to accomplish a task. The TDD cycle is red-green-refactor(?) and sure, you could hardcode the handling of a one-length input and an n-length input to make it green, but then there is still the third phase: To check if there is any need to improve the code. If someone just hardcodes statements to pass several tests, then I dare to say this person might not want to program in the first place.

My intent with the two test cases was, that it is easier to handle a single character input than a sequence. I think the handling of a sequence is an extension of the single char input. Small steps with fast achievements (in form of another green test) seem to be a big motivator to me, especially for inexperienced developers who have to think hard for every step they take.

I can't see the code context for this comment any more. So I can't comment further on that. (Force-pushing to branches under discussion is a bad idea.)

The TDD cycle is red-green-refactor(?) and sure, you could hardcode the handling of a one-length input and an n-length input to make it green, but then there is still the third phase: To check if there is any need to improve the code. If someone just hardcodes statements to pass several tests, then I dare to say this person might not want to program in the first place.

I disagree with this assessment.

If hardcoded values make the tests pass, then hardcode the values. The refactoring that needs to happen is improving the design of the existing code without altering its behaviour. If after "refactoring" your code can handle more cases than it could previously, you've strayed from the path.

The solution is to add more tests to test the cases that your code does not handle, and then write the code to pass these tests, and then to refactor to improve the design, again without altering its behaviour.

@Insti Oh. The context disappeared even after I committed normally. But you can see the context when viewing the first commit directly:
b024822#diff-054c8714b8d69933bb3aed926d01ea6bR32

But it is also in the second commit as well as the overview:
https://github.com/exercism/problem-specifications/pull/895/files#diff-054c8714b8d69933bb3aed926d01ea6bR41

Sorry if I did articulated myself a bit unclear. My paragraph you quoted actually does mean exactly what you argued about: One test at a time, it might be hardcoded at first, but at the refactoring step you should make sure that code smells get cleaned up. That's what I meant with "using the brain".

You argued:

Why is returning a hardcoded 1 different from returning a hardcoded 7?
And here my reply was that: There isn't, but only if you omit the refactoring part, which is an integral part of the cycle.

So my suggestion still stands:
Adding a test for a single-character input
and following up a test for a multi-char input.

reasoning:

motivation by fast rewards, in form of making implementation steps between tests smaller.

better implementation progression (sliming), because handling a single char is a subset of handling a sequence.

Adding a test for a single-character input
and following up a test for a multi-char input.

This is a good idea.

Vankog · 2017-09-08T18:19:34Z

exercises/nucleotide-count/canonical-data.json

+          "property": "nucleotideCounts",
+          "strand": "AGCTTTTCATTCTGACTGCAACGGGCAATATGTCTCTGTGTGGATTAAAAAAAGAGTGTCTGATAGCAGC",
+          "expected" : true
+        },


This is a case that I actually took over from the concrete JS implementation. I thought it is an interesting thought and it actually might occur.

This is a difficult thing to specify in this json file format.
I think it's better left out of the canonical data and implemented at the track level.

I agree with you. But how do you make sure it gets implemented in the tracks?

Limiting the possibilities of testing, just to fit into a constrained model or framework is generally a bad idea, though.
In this case I suggest we should think about a solution to this constraint in the first place (in another discussion of course). In the meantime, as a walkaround, what arguments speak against utilizing descriptions and comments to specify the case? How many tracks use test generators and how many do so blindly?

You might want to add a comment at the top of the file, suggesting a check for referential transparency. But I don't think it should have a separate test case.

how do you make sure it gets implemented in the tracks?

That is not something we need to or should worry about.

The people maintaining tracks are experienced developers with knowledge of their language, who have spent time reviewing many of the different solutions to exercises, when they identify solutions that are deficient in some way test cases can be added to the track. If these changes are thought to be more generally applicable they can then be added to this general repository.

The test case you are proposing is very implementation dependent and makes no sense in languages with immutable data. So for that reason I do not believe it is necessary to include it in the canonical data.

OK, I agree with this point about immutable languages. I'll remove it from the canonical data.

(One could argue now that some cases might be included in the canonical data as suggestions, kind of optional and meant to be mnemonic. Maybe in forms of comments. But this is another topic.)

Vankog · 2017-09-08T18:20:06Z

exercises/nucleotide-count/canonical-data.json

+        },
+        {
+          "description": "handle incorrect inputs",
+          "cases": [


added a separate case suite for error handling.

See comments on error case handling on your panagram PR.

Yes. I only included error handling of ASCII chars.

Insti · 2017-09-08T19:13:30Z

exercises/nucleotide-count/canonical-data.json

  "cases": [
    {
-      "description": "count all nucleotides in a strand",
+      "description": "nucleotideCounts counts all DNA-nucleotides in the input strand and returns an object with properties A, C, G and T holding their respective amounts.",


This does not need to be the description of the problem, that is provided in a separate file.

Several tracks generate the test name based on the test descriptions in this file so having a concise description is a good thing.

I see, thanks. I'll change this.

Insti · 2017-09-08T19:14:13Z

exercises/nucleotide-count/canonical-data.json

      "cases": [
        {
-          "description": "empty strand",
+          "description": "counts an empty DNA strand as 0.",


The original name here was fine, you are adding redundant information here.

You are right. Though, my intent was to implement well-formed descriptions in the informal style of
{it / the test} does xyz in the case of abc.
What I've experienced so far, this is not even enough. A test case (in TDD especially) acts as a specification. There are best practices (in some x-driven-development movements even rules) that suggest a test description should have several parts:

in case of x

then y

reasoning

some also want a precondition (like BDDs GIVEN WHEN THEN)

Just stating "empty string" as description tells a developer absolutely nothing about what should happen (or even why). I know you care about the reasoning part the same way I do, so I am not sure why you want to omit it here.

So in my eyes my suggested description might even be phrased like:
"Given an empty input string, it returns zeroes for all nucleotides."
While the reasoning for this behaviour results from the description of the test suite, because there is no need to state this dogmatically in every description.

"counts an empty DNA strand as 0."

'counts an' is redundant because all function calls will "count"

'DNA' is redundant because all input is DNA strands.

'as 0.' is information already present in the expected output.

Which leaves us with "empty strand"

Which means: "Returns the correct value when provided with an empty strand as input"

my intent was to implement well-formed descriptions in the informal style of
{it / the test} does xyz in the case of abc.

If a description can be generated from the data within the test it adds no value.
A descriptions should describe the "why" of the test. What does this test test that all the others do not?

I see it this way:

When running a test suite, you don't see the data, but only the names. So if for example something fails, then the test name itself should already give a clue about what might be the error or the constraints of the error.

When a user implements code inside an exercise track from a test spec or does bug fixing, he/she sees the data of the spec. But this only helps with trivial test logic. Say you have some more complex test logic, even though its intention can be described rather specifically in a sentence. Then why not being specific in the first place?

When a track maintainer implements a test from the canonical data, he/she sees the name and the data. But to create a meaningful test case he/she has to be able to know exactly what the test is about. Inferring things implicitly leads more often than not to misunderstandings. ("Oh, you meant this? I thought you meat that?") I think every programmer knows what it means when the specifications are unclear or when the customer has a different understanding than the developers. That's the reason why whole methodologies like Domain Driven Design evolved in the first place. I can't think of any requirement engineer who ever said: "Meh, no need to be redundant with this spec, this is obvious."

When you just say "empty strand", noone could infer...

"Returns the correct value when provided with an empty strand as input"

... unambiguously without context. It might as well mean that an empty strand should be returned.

Yes, in this particular context here it might be obvious, but e.g. in a bit more complex production code it ain't.

So the question that comes into my mind is:
Why is redundancy here in the canonical data considered a bad thing?
Don't get me wrong, being concise is nice and logical and good practice indeed. But is it pragmatic from the users or implementers perspective? Is it readable and most importantly understandable and unambiguous? Does a beginning learner already has the knowledge/skill to infer the meaning from the context?
For example: I just recently saw someone learning the very basics of programming from a nicely done beginners website. And he struggled with the easiest and basic tasks where everyone else with a bit more understanding would knew what was going on in an instant. But he was not fully aware of the context that was given and therefore could not infer it easily.

So the TL;DR ^^:
Why do you consider redundancy in a spec file (more so even meta-spec file) a bad thing if it helps making the matter more clear and unambiguous?

When running a test suite, you don't see the data, but only the names.

You need a better test runner. I see the expected input and output values and highlighted diffs of the results.

this only helps with trivial test logic. Say you have some more complex test logic

I'm not sure what your point is in this paragraph, but if your test logic is complex you're doing something wrong.

to create a meaningful test case he/she has to be able to know exactly what the test is about

Several tracks implement test generators that automatically generate the tests without the track maintainer needing to look at any of the tests, so I would argue this statement is false.

Why do you consider redundancy in a spec file (more so even meta-spec file) a bad thing if it helps making the matter more clear and unambiguous?

Redundancy in a description is often a sign that the description is not telling me why this test is important, but is just repeating what the inputs and outputs are.

If it really is making things clear an unambiguous is it really redundant?
(see: no true Scotsman)

Insti · 2017-09-08T19:21:22Z

exercises/nucleotide-count/canonical-data.json

          "property": "nucleotideCounts",
-          "strand": "AGXXACT",
+          "strand": "aGcTtTtCaTtCtGaCtGcAaCgGgCaAtAtGtCtCtGtGtGgAtTaAaAaAaGaGtGtCtGaTaGcAgC",


This exercise is not about case comparisons. We should be able to trust that all the input characters are valid.

I respectfully disagree. See my reasoning above (#895 (comment)).

We educate people about programming. Teaching them what it means to test for all equivalence classes (as they are called) should be an integral part of this progress right from the beginning.

Why should we assume that all inputs are valid? This works to a certain degree in one-man projects and quick (and/or dirty) implementations, but this is nothing we should promote. In reality murphy's law rules.

I know that several test cases seem tedious to write, but why bother here? The users don't even write the tests, we do. But we should be a good example.
And implementing such minor specifications as the exercise users, does not seem to be a big issue to me. They are not complex, but integral. I had a look at several submissions to the exercise and most users normalize case anyway. So why not making it explicit?

In many cases when developing, it is perfectly safe to assume that an input is valid. You may for example have integration test coverage proving that invalid input never reaches your code under unit test.

Validations may be redundant, and cost both programmer time and CPU cycles.

I agree, we should avoid redundant tests.
However, we don't have such (integration-) tests in place.

There are also no tests that the keyboard is plugged in or the screen is turned on.

Now you are dragging the matter into ridiculousness. Sorry, but this is not a valid reasoning but only a killer argument. ;-)

Now you are dragging the matter into ridiculousness.

Guilty. 😊

However, we don't have such (integration-) tests in place.

Is this not guilty of the same thing?
That we don't have integration tests is irrelevant because we do not need them, we can trust that the test input is correct and valid.

To a degree, yes.
But I think we should continue this thread over at the discussion thread.

Insti

See individual comments.

This attempts to improve the BDD/TDD progression ("sliming"), better descriptions, added test cases. exercism#895

exercism#895

Vankog · 2017-09-15T17:00:49Z

@Insti Thanks for your feedback.
I have replied to them individually above.
For now I changed the descriptions again.

Vankog · 2017-09-15T17:03:55Z

exercises/nucleotide-count/canonical-data.json

+      "comments": [
+        "nucleotideCounts counts all DNA-nucleotides in the input strand and returns an object with properties",
+        "A, C, G and T holding their respective amounts."
+      ],


I moved the longer description into a comment like others do it.

petertseng · 2017-09-16T18:06:14Z

exercises/nucleotide-count/canonical-data.json

+            {
+              "description": "throws error if DNA input does not include anything but ACGT-characters.",
+              "property": "nucleotideCounts",
+              "strand": "AGUUÄ1+ :'A'}CT",


can you confirm whether or not Ä is in ASCII, given (I think) a declaration of intent in #895 (comment)

I only included error handling of ASCII chars.

(Let me know if I simply misinterpreted the declaration)

Yep, Ä is ASCII: :-)
http://www.theasciicode.com.ar/extended-ascii-code/letter-a-umlaut-diaeresis-a-umlaut-lowercase-ascii-code-132.html

(Btw: ß too, wich was part of the Unicode-discussion ;-) )

Ä is an upper ASCII character. Is the intent to restrict to printable characters? Ä (142) is not considered a printable character.

@rpottsoh Printable in what context?
In ASCII everything is considered printable from 32-255 (excluding 127).

In the context of this exercise only aA,cC,gG,tT is considered valid.
In another exercise (e.g. pangram) it might be everything from a-zA-Z.
In other cases even everything technically printable, might be valid. It really depends.

The issue discussed here was just, whether it is a Unicode-exclusive character like ℣ or 💩, because there is consensus not to check them.

Chars 32 through 126 are considered printable. 128 through 255 are considered extended.

You can't grep UTF8 encoded files assuming ASCII char numbers

I'm sorry, I don't understand. The specific grep I performed (grep AGUU) doesn't affect the bytes in the files; it simply makes it so that the only line that xxd displays is the line with the bytes we are discussing, since I grepped for the characters immediately before. If I need to use grep a different way, can I ask if you know any proper flags I need to pass to grep?

Alternatively, if you were referring to the other grep I mentioned (git grep -Pnl "[\x80-\xFF]") then I acknowledge that the specific characters found by it will be dependent on encoding; if we need a different pattern, I will discuss and find in #428.

This is what you xxd with the file encoded in ANSI
To use xxd, you actually have to couple it with an UTF8-aware text viewer

I'm sorry, I don't understand either of these two assertions. I thought xxd simply shows me the bytes in the file. Can I ask you do one of the two following?

Show me different flags xxd on canonical-data.json which demonstrates the three possibilities you report (reproduced below)

6E 64 22 3A 20 22 41 47 55 55 C3 84 31 2B 20 3A // UTF8 6E 64 22 3A 20 22 41 47 55 55 8E 31 2B 20 3A 27 // ASCII 6E 64 22 3A 20 22 41 47 55 55 C4 31 2B 20 3A 27 // ANSI

OR show me any other way of reading the exact bytes in canonical-data.json that demonstrates that the byte(s) that you desire exist in the file?

What I thought I did was to report to you the bytes as they lie in this file on my hard drive as they were delivered to me by Git. Is that not what I did?

Why am I asking to be shown? Simply so that I know that you are OK with the bytes that exist in this file (0xc3 0x84), and they match your intention.

Sorry for the grep confusion. I thought you xxd'ed and grep'ed afterwards. I am on Windows, so I am not that firm with unix commands.

Alternatively, if you were referring to the other grep I mentioned (git grep -Pnl "[\x80-\xFF]") then I acknowledge that the specific characters found by it will be dependent on encoding; if we need a different pattern, I will discuss and find in #428.

Jup, that's what I meant.

Let me say it in other words:
When you view the bytecode of the file now as-is from git, it will show you what you already reported:
6E 64 22 3A 20 22 41 47 55 55 -> C3 84 <- 31 2B 20 3A
However, the file has no separate ASCII characters C3 and 84 in it, but has a single character C384, because it is UTF8-encoded.

If I would have encoded the same text content of the file in ASCII, though, and sent it to you, you would see the following byte code:
6E 64 22 3A 20 22 41 47 55 55 -> 8E <- 31 2B 20 3A 27
This was just included by me for demonstration purposes.

But I figure we had a bit of a different understanding, what the conclusion of #428 meant.
Insti's concluding post says:

All test cases should only use ASCII characters

In my interpretation this meant all ASCII, including the extended ASCII (8-bit).
In contrast, I think you interpreted it as the 7-bit ASCII only, didn't you?
So no wonder why you and I am confused about each other. ^^

However, the file has no separate ASCII characters C3 and 84 in it, but has a single character C384, because it is UTF8-encoded.

I agree with this statement. The file does not contain the characters 0xC3 and 0x84. It contains the bytes 0xC3 and 0x84, which together represent Unicode codepoint 196 (Feel free to check my math: 0xc3 is 11000011 and 0x84 is 10000100 so we concatenate 00011 and 000100 to get 11000100 which is 196). So we seem to be on the same page with this, good.

If I would have encoded the same text content of the file in ASCII, though, and sent it to you, you would see the following byte code:
6E 64 22 3A 20 22 41 47 55 55 -> 8E <- 31 2B 20 3A 27

Got it.

Yep, we are on the same page. :-)

I didn't see that you had edited your post.

All test cases should only use ASCII characters

In my interpretation this meant all ASCII, including the extended ASCII (8-bit).
In contrast, I think you interpreted it as the 7-bit ASCII only, didn't you?

It is true that given that I grepped for bytes 0x80 to 0xFF that I looked for files that had characters outside of 7-bit ASCII. I wouldn't know for sure what the original text meant, but I know I got no objections at the time, so that is moderate evidence in favour of 7-bit ASCII. If it does in fact mean 7-bit ASCII, it would imply needing to remove the Ä. I'll leave that decision to the interested parties.

petertseng · 2017-09-16T18:10:45Z

exercises/nucleotide-count/canonical-data.json

+          ],
+          "description": "returns zeroes for an undefined/null strand as argument.",
+          "property": "nucleotideCounts",
+          "strand": null,


If null is tested as an input, I am thinking it should be an error, rather than to be treated as if "" was the input. The reason for this belief is that if we consider that the function under test must only accept strings, well null is not a string, so this is in error.

(Of course, I imagine languages for which strings are non-nullable will simply skip this test)

This comment must not be read as an endorsement or rejection of the statement "null should be tested as an input".

Yeah, I agree. I had this same thought but had not changed it, yet, because of the ongoing discussion. I will probably change it to expect a throw in an upcoming commit.

stevejb71 · 2017-09-17T04:55:05Z

exercises/nucleotide-count/canonical-data.json

+          "description": "handles incorrect inputs.",
+          "cases": [
+            {
+              "description": "throws error if DNA input does not include anything but ACGT-characters.",


Some languages do not have exceptions, or would return an error type for invalid inputs.

But this is not an issue with this canonical data.
"error" is explicitly described in the readme:
https://github.com/exercism/problem-specifications#test-data-format-canonical-datajson
and also in the schema.

Or do you just mind the wording of the description: "throws error"?
Not sure how to describe this instead.

I simply object to the use of the word "throws", because it is not language general enough. Instead, you might write "gives an error if.." or "xxx input is invalid".

@stevejb71 thanks!
If you don't mind, there is also a discussion about this over here: #905

Insti · 2017-10-01T19:12:24Z

@Vankog are you interested in continuing to work on these pull requests?
I think they have achieved their goals of stimulating discussion, but still require further work to be considered ready to merge.

(If no further progress is made, I will close this on or after the 8th October.)

Vankog · 2017-10-02T10:53:26Z

I do. I was just waiting for the sub-discussions to conclude so we can finally decide what should or shouldn't be done.

This attempts to improve the BDD/TDD progression ("sliming"), better descriptions, added test cases. exercism#895

exercism#895

Vankog · 2017-10-06T15:06:15Z

OK, new suggestion.

converted the controversial cases into comments to support the track maintainers with this task.
shortened/tweaked some test descriptions a bit.
removed the ASCII >126 character.

petertseng

I think the added one-letter case makes sense.

In my ideal world, there would be one commit that changes descriptions of all existing test cases, and a second commit that adds new cases. I will not be able to force myself to Approve (in the GitHub) other separations of commits, but I do not object if others Approve.

petertseng · 2017-10-13T03:10:40Z

exercises/nucleotide-count/canonical-data.json

+    "input error handling (null/undefined parameters etc.)",
+    "referential transparency (i.e. evaluating, a function/method gives the same value for same arguments every time.)",
+    "",
+    "'error' and 'null' should be treated according to the languages' specifics and possibilities."


since there is no more null that I see (I could have missed), the part about null wouldn't be necessary

petertseng · 2017-10-13T03:12:25Z

exercises/nucleotide-count/canonical-data.json

          "property": "nucleotideCounts",
-          "strand": "AGXXACT",
+          "strand": "aGcTtTtCaTtCtGaCtGcAaCgGgCaAtAtGtCtCtGtGtGgAtTaAaAaAaGaGtGtCtGaTaGcAgC",


I'm not convinced it's necessary to add the extra functionality (of ignoring case), but I also don't care.

petertseng · 2017-10-13T03:15:06Z

exercises/nucleotide-count/canonical-data.json

+            {
+              "description": "treats an input with anything else than ACGT-characters as error.",
+              "property": "nucleotideCounts",
+              "strand": "AGUU1+ :'A'}CT",


I don't think this case is any better or worse than "AGXXACT" at ensuring the description is true. I might even call it worse just for clarity. I would keep AGXXACT. Strength of opinion 2/10.

Insti · 2017-10-13T06:22:52Z

I agree with @petertseng

As a reviewer this PR has gotten to a stage where reviewing it is very difficult.
I still agree with some of the changes and disagree with others, but reviewing all the different threads and working out what has changed in amongst all the changes and comments will take more time than I have available to review, so I do not expect I will change my my review status from changes requested.

I recommend this PR be closed and new separate PRs be opened for the different changes.

This attempts to improve the BDD/TDD progression ("sliming"), better descriptions, added test cases. exercism#895

exercism#895

Vankog · 2017-10-13T13:11:57Z

tasks:

adding single char input case ~~nucleotide-count: Adding a single character case #951~~
bug: fix incorrect order of single-input <-> multi-input cases ~~nucleotide-count: fix wrong order introduced in #951 #957~~
adding char-case-agnostic case nucleotide-count: add case-agnostic test #959
extend "strand with invalid nucleotides" case
add suite comments (incl. suggestions for track maintainers)
improve case descriptions

Insti · 2017-10-14T12:16:55Z

Since this is being split into new separate PRs I think we can close this one.
Thanks for starting the discussion @Vankog ❤️

This attempts to improve the BDD/TDD progression ("sliming"), better descriptions, added test cases. exercism#895

exercism#895

This attempts to improve the BDD/TDD progression ("sliming"), better descriptions, added test cases. exercism#895

exercism#895

Vankog force-pushed the refactor-nucleotide-count-test branch from fae167a to 1000b98 Compare September 8, 2017 18:07

Vankog added a commit to Vankog/problem-specifications that referenced this pull request Sep 8, 2017

refactored the nucleotide-count test data

1000b98

This attempts to improve the BDD/TDD progression ("sliming"), better descriptions, added test cases. exercism#895

Vankog mentioned this pull request Sep 8, 2017

Please review: Implemented exercise nucleotide-count exercism/javascript#349

Closed

Vankog commented Sep 8, 2017

View reviewed changes

Insti reviewed Sep 8, 2017

View reviewed changes

Insti suggested changes Sep 8, 2017

View reviewed changes

Insti changed the title ~~discussion: refactoring nucleotide-count test~~ nucleotide-count: refactoring tests (discussion) Sep 11, 2017

Insti added the discussion label Sep 11, 2017

Vankog mentioned this pull request Sep 15, 2017

pangram: rework tests (discussion) #893

Closed

Vankog added a commit to Vankog/problem-specifications that referenced this pull request Sep 15, 2017

refactored the nucleotide-count test data

41e55f5

This attempts to improve the BDD/TDD progression ("sliming"), better descriptions, added test cases. exercism#895

Vankog force-pushed the refactor-nucleotide-count-test branch 2 times, most recently from 1000b98 to b024822 Compare September 15, 2017 16:17

Vankog added a commit to Vankog/problem-specifications that referenced this pull request Sep 15, 2017

refactored the nucleotide-count test data

b024822

This attempts to improve the BDD/TDD progression ("sliming"), better descriptions, added test cases. exercism#895

Vankog added a commit to Vankog/problem-specifications that referenced this pull request Sep 15, 2017

refining nucleotide-count test after feedback:

3167e3c

exercism#895

Vankog commented Sep 15, 2017

View reviewed changes

Insti mentioned this pull request Sep 16, 2017

Discussion: Testing 'invalid' input. #902

Open

petertseng reviewed Sep 16, 2017

View reviewed changes

stevejb71 suggested changes Sep 17, 2017

View reviewed changes

Vankog mentioned this pull request Sep 17, 2017

Discussion: Getting rid of 'null' and "error" in canonical-data? #905

Open

Vankog force-pushed the refactor-nucleotide-count-test branch from 3167e3c to 0366e07 Compare October 6, 2017 14:02

Vankog added a commit to Vankog/problem-specifications that referenced this pull request Oct 6, 2017

refactored the nucleotide-count test data

dc2239e

This attempts to improve the BDD/TDD progression ("sliming"), better descriptions, added test cases. exercism#895

Vankog added a commit to Vankog/problem-specifications that referenced this pull request Oct 6, 2017

refining nucleotide-count test after feedback:

0366e07

exercism#895

Vankog added a commit to Vankog/problem-specifications that referenced this pull request Oct 6, 2017

new revision of nucleotide count

ba6b7b1

exercism#895

m-dango mentioned this pull request Oct 6, 2017

Add nucleotide-count exercise exercism/raku#200

Merged

petertseng reviewed Oct 13, 2017

View reviewed changes

Vankog added 3 commits October 13, 2017 14:30

refactored the nucleotide-count test data

7bd8580

This attempts to improve the BDD/TDD progression ("sliming"), better descriptions, added test cases. exercism#895

refining nucleotide-count test after feedback:

4eb42d7

exercism#895

new revision of nucleotide count

b22ce4a

exercism#895

Vankog force-pushed the refactor-nucleotide-count-test branch from ba6b7b1 to b22ce4a Compare October 13, 2017 12:30

Vankog mentioned this pull request Oct 13, 2017

nucleotide-count: Adding a single character case #951

Merged

Insti closed this Oct 14, 2017

Vankog added a commit to Vankog/problem-specifications that referenced this pull request Oct 16, 2017

refactored the nucleotide-count test data

4cdeb69

This attempts to improve the BDD/TDD progression ("sliming"), better descriptions, added test cases. exercism#895

Vankog added a commit to Vankog/problem-specifications that referenced this pull request Oct 16, 2017

refining nucleotide-count test after feedback:

ca1a46e

exercism#895

Vankog added a commit to Vankog/problem-specifications that referenced this pull request Oct 16, 2017

new revision of nucleotide count

e8989a0

exercism#895

Vankog added a commit to Vankog/problem-specifications that referenced this pull request Oct 17, 2017

refactored the nucleotide-count test data

ec199fd

This attempts to improve the BDD/TDD progression ("sliming"), better descriptions, added test cases. exercism#895

Vankog added a commit to Vankog/problem-specifications that referenced this pull request Oct 17, 2017

refining nucleotide-count test after feedback:

337423d

exercism#895

Vankog added a commit to Vankog/problem-specifications that referenced this pull request Oct 17, 2017

new revision of nucleotide count

3d7f067

exercism#895

This was referenced Oct 17, 2017

nucleotide-count: add case-agnostic test #959

Closed

Port nucleotide-count exercise from JavaScript track exercism/javascript#361

Closed

nucleotide-count: refactoring tests (discussion) #895

nucleotide-count: refactoring tests (discussion) #895

Conversation

Vankog commented Sep 8, 2017

Background:

Analysis:

Suggestion:

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Vankog Sep 15, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Vankog Sep 8, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Vankog Sep 15, 2017 • edited Loading

Choose a reason for hiding this comment

Insti Sep 16, 2017 • edited Loading

Choose a reason for hiding this comment

Vankog Sep 16, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Vankog Sep 16, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Vankog Sep 15, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Vankog Sep 16, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Vankog Sep 15, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Vankog Sep 16, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Insti left a comment

Choose a reason for hiding this comment

Vankog commented Sep 15, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

petertseng Sep 17, 2017 • edited Loading

Choose a reason for hiding this comment

Vankog Sep 17, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

petertseng Sep 16, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Insti commented Oct 1, 2017

Vankog commented Oct 2, 2017

Vankog commented Oct 6, 2017

petertseng left a comment

Vankog Sep 15, 2017 •

edited

Loading

Vankog Sep 8, 2017 •

edited

Loading

Vankog Sep 15, 2017 •

edited

Loading

Insti Sep 16, 2017 •

edited

Loading

Vankog Sep 16, 2017 •

edited

Loading

Vankog Sep 16, 2017 •

edited

Loading

Vankog Sep 15, 2017 •

edited

Loading

Vankog Sep 16, 2017 •

edited

Loading

Vankog Sep 15, 2017 •

edited

Loading

Vankog Sep 16, 2017 •

edited

Loading

petertseng Sep 17, 2017 •

edited

Loading

Vankog Sep 17, 2017 •

edited

Loading

petertseng Sep 16, 2017 •

edited

Loading

Vankog commented Oct 13, 2017 •

edited

Loading