Skip to content

Check output to include missing test case descriptions #44

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 2 commits into from
Oct 16, 2020
Merged

Check output to include missing test case descriptions #44

merged 2 commits into from
Oct 16, 2020

Conversation

glennj
Copy link
Contributor

@glennj glennj commented Oct 15, 2020

Sample output, from the current state of the Tcl track:

$ ../canonical-data-syncer/canonical_data_syncer -c
Checking exercises...
... git clone output ...
[warn] list-ops: missing 1 test cases
       - empty list to list (e842efed-3bf6-4295-b371-4d67a4fdf19c)
[warn] some exercises are missing test cases

@glennj
Copy link
Contributor Author

glennj commented Oct 15, 2020

I logged the missing case descriptions with logNormal -- I was wondering if logDetailed might be more appropriate, but then they disappear into all the skip messages.

@ErikSchierboom
Copy link
Member

I like the addition!

I logged the missing case descriptions with logNormal -- I was wondering if logDetailed might be more appropriate, but then they disappear into all the skip messages.

Maybe it would make sense to introduce another log level, as the skip messages are almost never that relevant except when doing some debugging. @ee7 what do you think?

@ee7
Copy link
Member

ee7 commented Oct 16, 2020

@glennj Nice PR, thanks. I also was thinking that we should show the missing test cases, because they aren't currently shown even in our most verbose output.

Maybe it would make sense to introduce another log level, as the skip messages are almost never that relevant except when doing some debugging.

Hmm, I hadn't considered that. It's possible, but I think 3 verbosity levels is a good number - maybe we don't want to cram e.g. --verbosity=full or --verbosity=debug into the --help message (or try to explain verbosity levels "1" to "4")?

Other things I thought of before are:

  1. Sort the output by category (missing tests/no canonical data/up-to-date) then alphabetically, rather than just alphabetically. Maybe we could even do this only with --verbosity=detailed. Or add an option to control the sorting.
  2. Add colors to help us distinguish between [warn] and [skip], or between missing/non-canonical/up-to-date.
  3. Use a table for the output.

I think I'd prefer to try out options 1 and 2 before adding another verbosity level. What do you think?

I logged the missing case descriptions with logNormal -- I was wondering if logDetailed might be more appropriate, but then they disappear into all the skip messages.

I think this PR is a strict improvement to the --verbosity=detailed, since it adds useful information that was missing from our most verbose output.

But I think we should optimize for usefulness and clarity at the default verbosity level. So the question is: is it generally helpful to show the missing test names to the user who writes just ./canonical_data_syncer -c? I think "yes", because:

  • the user is generally a maintainer
  • it should be rare that the output contains a huge number of missing tests. And if it does, I'd still want to see it all in one place.

Therefore I consider this PR an improvement to the default verbosity level too, and I think we can merge and discuss other improvements later.

Just to summarise this PR, let's consider some track that has two exercises with missing test cases.

Default verbosity

Before this PR:

$ ./canonical_data_syncer -c
[...]
[warn] acronym: missing 3 test cases
[warn] grains: missing 2 test cases
[warn] some exercises are missing test cases

With this PR:

$ ./canonical_data_syncer -c
[...]
[warn] acronym: missing 3 test cases
       - very long abbreviation (0e4b1e7c-1a6d-48fb-81a7-bf65eb9e69f9)
       - consecutive delimiters (6a078f49-c68d-4b7b-89af-33a1a98c28cc)
       - apostrophes (5118b4b1-4572-434c-8d57-5b762e57973e)
[warn] grains: missing 2 test cases
       - grains on square 3 (10f45584-2fc3-4875-8ec6-666065d1163b)
       - grains on square 4 (a7cbe01b-36f4-4601-b053-c5f6ae055170)
[warn] some exercises are missing test cases

With --verbosity=detailed

I don't trim the output below, so we can see how much the [warn] can disappear into the [skip].

Before this PR (good luck seeing grains and robot-name!):

$ ./canonical_data_syncer --verbosity=detailed
[...]
[warn] acronym: missing 3 test cases
[skip] all-your-base: up-to-date
[skip] allergies: up-to-date
[skip] anagram: up-to-date
[skip] armstrong-numbers: up-to-date
[skip] atbash-cipher: up-to-date
[skip] binary: up-to-date
[skip] bob: up-to-date
[skip] clock: up-to-date
[skip] collatz-conjecture: up-to-date
[skip] crypto-square: up-to-date
[skip] darts: up-to-date
[skip] diamond: up-to-date
[skip] difference-of-squares: up-to-date
[skip] diffie-hellman: up-to-date
[skip] etl: up-to-date
[skip] gigasecond: up-to-date
[skip] grade-school: up-to-date
[warn] grains: missing 2 test cases
[skip] hamming: up-to-date
[skip] hello-world: up-to-date
[skip] high-scores: up-to-date
[skip] isbn-verifier: up-to-date
[skip] isogram: up-to-date
[skip] kindergarten-garden: up-to-date
[skip] largest-series-product: up-to-date
[skip] leap: up-to-date
[skip] luhn: up-to-date
[skip] matching-brackets: up-to-date
[skip] matrix: up-to-date
[skip] meetup: up-to-date
[skip] nth-prime: up-to-date
[skip] nucleotide-count: up-to-date
[skip] pangram: up-to-date
[skip] pascals-triangle: up-to-date
[skip] perfect-numbers: up-to-date
[skip] phone-number: up-to-date
[skip] prime-factors: up-to-date
[skip] protein-translation: up-to-date
[skip] proverb: up-to-date
[skip] queen-attack: up-to-date
[skip] raindrops: up-to-date
[skip] react: up-to-date
[skip] resistor-color: up-to-date
[skip] resistor-color-duo: up-to-date
[skip] resistor-color-trio: up-to-date
[skip] reverse-string: up-to-date
[skip] rna-transcription: up-to-date
[skip] robot-name: does not have canonical data
[skip] roman-numerals: up-to-date
[skip] rotational-cipher: up-to-date
[skip] run-length-encoding: up-to-date
[skip] saddle-points: up-to-date
[skip] say: up-to-date
[skip] scale-generator: up-to-date
[skip] scrabble-score: up-to-date
[skip] secret-handshake: up-to-date
[skip] series: up-to-date
[skip] sieve: up-to-date
[skip] space-age: up-to-date
[skip] spiral-matrix: up-to-date
[skip] sublist: up-to-date
[skip] sum-of-multiples: up-to-date
[skip] triangle: up-to-date
[skip] twelve-days: up-to-date
[skip] two-fer: up-to-date
[skip] word-count: up-to-date
[skip] yacht: up-to-date
[warn] some exercises are missing test cases

With this PR:

$ ./canonical_data_syncer -c --verbosity=detailed
[...]
[warn] acronym: missing 3 test cases
       - very long abbreviation (0e4b1e7c-1a6d-48fb-81a7-bf65eb9e69f9)
       - consecutive delimiters (6a078f49-c68d-4b7b-89af-33a1a98c28cc)
       - apostrophes (5118b4b1-4572-434c-8d57-5b762e57973e)
[skip] all-your-base: up-to-date
[skip] allergies: up-to-date
[skip] anagram: up-to-date
[skip] armstrong-numbers: up-to-date
[skip] atbash-cipher: up-to-date
[skip] binary: up-to-date
[skip] bob: up-to-date
[skip] clock: up-to-date
[skip] collatz-conjecture: up-to-date
[skip] crypto-square: up-to-date
[skip] darts: up-to-date
[skip] diamond: up-to-date
[skip] difference-of-squares: up-to-date
[skip] diffie-hellman: up-to-date
[skip] etl: up-to-date
[skip] gigasecond: up-to-date
[skip] grade-school: up-to-date
[warn] grains: missing 2 test cases
       - grains on square 3 (10f45584-2fc3-4875-8ec6-666065d1163b)
       - grains on square 4 (a7cbe01b-36f4-4601-b053-c5f6ae055170)
[skip] hamming: up-to-date
[skip] hello-world: up-to-date
[skip] high-scores: up-to-date
[skip] isbn-verifier: up-to-date
[skip] isogram: up-to-date
[skip] kindergarten-garden: up-to-date
[skip] largest-series-product: up-to-date
[skip] leap: up-to-date
[skip] luhn: up-to-date
[skip] matching-brackets: up-to-date
[skip] matrix: up-to-date
[skip] meetup: up-to-date
[skip] nth-prime: up-to-date
[skip] nucleotide-count: up-to-date
[skip] pangram: up-to-date
[skip] pascals-triangle: up-to-date
[skip] perfect-numbers: up-to-date
[skip] phone-number: up-to-date
[skip] prime-factors: up-to-date
[skip] protein-translation: up-to-date
[skip] proverb: up-to-date
[skip] queen-attack: up-to-date
[skip] raindrops: up-to-date
[skip] react: up-to-date
[skip] resistor-color: up-to-date
[skip] resistor-color-duo: up-to-date
[skip] resistor-color-trio: up-to-date
[skip] reverse-string: up-to-date
[skip] rna-transcription: up-to-date
[skip] robot-name: does not have canonical data
[skip] roman-numerals: up-to-date
[skip] rotational-cipher: up-to-date
[skip] run-length-encoding: up-to-date
[skip] saddle-points: up-to-date
[skip] say: up-to-date
[skip] scale-generator: up-to-date
[skip] scrabble-score: up-to-date
[skip] secret-handshake: up-to-date
[skip] series: up-to-date
[skip] sieve: up-to-date
[skip] space-age: up-to-date
[skip] spiral-matrix: up-to-date
[skip] sublist: up-to-date
[skip] sum-of-multiples: up-to-date
[skip] triangle: up-to-date
[skip] twelve-days: up-to-date
[skip] two-fer: up-to-date
[skip] word-count: up-to-date
[skip] yacht: up-to-date
[warn] some exercises are missing test cases

@ErikSchierboom
Copy link
Member

Sort the output by category (missing tests/no canonical data/up-to-date) then alphabetically, rather than just alphabetically. Maybe we could even do this only with --verbosity=detailed. Or add an option to control the sorting.
Add colors to help us distinguish between [warn] and [skip], or between missing/non-canonical/up-to-date.
Use a table for the output.

My preference would then be to use colors to help distinguish the output. I'm not in favor of adding sorting options and I'm unsure about the categories. A table could also be nice.

@ErikSchierboom ErikSchierboom merged commit 961385f into exercism:master Oct 16, 2020
@ErikSchierboom
Copy link
Member

Merged and a release (0.10.0) is being built!

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