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

crypto-square: tests are implementation specific. #422

Closed
Insti opened this issue Aug 28, 2016 · 7 comments · Fixed by #738
Closed

crypto-square: tests are implementation specific. #422

Insti opened this issue Aug 28, 2016 · 7 comments · Fixed by #738

Comments

@Insti
Copy link
Contributor

Insti commented Aug 28, 2016

The conclusion reached in Intermediate functions: To test or not to test? was that intermediate functions should not be tested.

The Ruby crypto-square tests are basically all intermediate function tests.

The x-common test data also includes tests for intermediate functions.

The Ruby tests and the x-common test data need to be rewritten to eliminate intermediate function testing.

Edit: to fix test data link

@nickborromeo
Copy link

@Insti wanted to help out with this particular issue. I read through all the links and the discussions that you all had however its still not that clear to me what intermediate functions are 😄

@Insti
Copy link
Contributor Author

Insti commented Sep 27, 2016

@nickborromeo ,

The readme says:

Write a program that, given an English text, outputs the encoded version of that text.

So the "acceptance" tests should be of the form:

assert_equal 'encodedtext', crypto.encode

However the tests that currently exist are full of tests for things like:

assert_equal '123go', crypto.normalize_plaintext
assert_equal 4, crypto.size
assert_equal %w(zomg zomb ies), crypto.plaintext_segments

normalize_plaintext, size, plaintext_segments etc. are the intermediate functions.

Having them in forces you to re-implement the same solution as the initial test writer, but they are not necessarily required to solve the problem which is just to do the required encoding.

@rbasso
Copy link

rbasso commented Oct 7, 2016

I don't know if it will help; but, after reading this issue, we came to a similar conclusion in exercism/haskell#259.

Because the intermediary tests where most of the test suite, we had to write new tests in exercism/haskell#391:

cases :: [Case]
cases = [ Case { description = "perfect square, all lowercase with space"
               , input       = "a dog"
               , expected    = "ao dg"
               }
        , Case { description = "perfect rectangle, mixed case"
               , input       = "A camel"
               , expected    = "am ce al"
               }
        , Case { description = "incomplete square with punctuation"
               , input       = "Wait, fox!"
               , expected    = "wtx af io"
               }
        , Case { description = "incomplete rectangle with symbols"
               , input       = "cat | cut -d@ -f1 | sort | uniq"
               , expected    = "ctoi adrq tft c1u usn"
               }
]

To keep a smooth progression, we are sequentially testing each of the outputs for the following properties:

  • Does the output have the expected non-space characters?
  • Does the output have the expected non-space characters in the correct order?
  • Does the output have the expected blocks?

@stale stale bot added the wontfix label Apr 29, 2017
@stale
Copy link

stale bot commented Apr 29, 2017

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@kotp kotp added the stale label May 1, 2017
@stale stale bot removed the stale label May 1, 2017
@kotp kotp added stale and removed wontfix labels May 1, 2017
@stale stale bot removed the stale label May 1, 2017
@stale stale bot added the stale label Jun 30, 2017
@stale
Copy link

stale bot commented Jun 30, 2017

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@Insti Insti removed the stale label Jun 30, 2017
@stale stale bot added the stale label Aug 29, 2017
@stale
Copy link

stale bot commented Aug 29, 2017

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@nholden
Copy link

nholden commented Oct 9, 2017

I'm wrapping up a couple of pull requests in problem-specifications to improve the canonical data and description. Once those are live, I'll submit a PR in this repo that reflects the changes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants