Migrate data-driven tests to subtests

[artamonovkirill]

Following this Slack thread and this PR discussion

In the track, there are still some data-driven tests like:

func TestSomething(t *testing.T) {
    for _, test := range tests {
        ... some test code ...
        t.Logf("PASS: %s", test.description)
    }
}

Such tests could benefit from rewriting them with subtests:

func TestSomething(t *testing.T) {
    for _, test := range tests {
        t.Run(test.description, func(t *testing.T) {
            ... some test code ...
        }
    }
}

Pros:

• Easier to parse test output of the test runner
• For an IDE user, it's easy to see which test failed without the need to scroll (sometimes) lengthy output.
• A possibility to run exactly 1 subtest

Cons:

• test names/descriptions lowercased with underscores instead of spaces
• no "fail fast" behaviour without checking the output value from t.Run

A (possibly incomplete) list of exercises without subtests:

• List Ops
• Run Length Encoding
• Change
• Variable Length Quantity
• Perfect Numbers
• Allergies
• Bowling
• Protein Translation
  ... (around 43 exercises in total - according to a search on t.Logf("PASS)

[artamonovkirill]

Slack thread screenshot, if it gets deleted:

[andrerfcsantos]

If anyone wants to work on this issue, it is OK (and even recommended) the make PR's for individual exercises - working on this issue doesn't mean necessarily fixing all exercises yourself all at once. On the contrary, several people are more than welcome to make PR's for individual exercises where this is a problem.

We'll use this issue as a tracking issue to track all the PR's related to this.

[junedev]

@andrerfcsantos Did you already check whether this issue might also be related to code that produced via the generator? I have a hunch this is not just a matter of making individual PRs but this might be a generator issues that needs to be addressed.

[andrerfcsantos]

@junedev Some exercises suffering from this problem do have generators, but others don't. So I don't think it's a generator specific problem, although for the exercises that do have the generator, we must adjust it. Your suggestion would be to add this capability to the generator first and then work on individual exercises? (Although exercises without a generator currently don't have this dependency)

[eklatzer]

Hey, to get an overview I used the following script:

#! /bin/bash

for test in ./exercises/*/*/*_test.go; do
    if [[ $test == *cases_test* ]] # ignore cases_test.go
    then
        continue
    fi
    testFileWithoutSubtest=$(grep -Li "t.Run" "${test}")
    if [ ! -z "$testFileWithoutSubtest" ]
    then
        echo "- [ ] ${testFileWithoutSubtest}"
    fi
done

Seems like the following exercises still need subtests (some might not need subtests):

• ./exercises/practice/two-bucket/bonus_test.go (fixed bonus tests #2437)
• ./exercises/concept/chessboard/chessboard_test.go (added subtests to chessboard #2428)
• ./exercises/practice/bowling/bowling_test.go (added usage of subtests for exercise bowling #2430)
• ./exercises/practice/crypto-square/crypto_square_test.go (added usage of subtests for exercise crypto-square #2431)
• ./exercises/practice/difference-of-squares/difference_of_squares_test.go (added usage of subtests to difference-of-squares #2432)
• ./exercises/practice/etl/etl_test.go (added usage of subtest for etl #2434)
• ./exercises/practice/food-chain/food_chain_test.go (added usage of subtests food-chain #2435)
• ./exercises/practice/house/house_test.go (added usage of subtests to the house exercise #2438)
• ./exercises/practice/kindergarten-garden/kindergarten_garden_test.go (added usage of subtests to the kindergarten-garden exercise #2439)
• ./exercises/practice/ledger/ledger_test.go (added subtests to the ledger exercise #2442)
• ./exercises/practice/list-ops/list_ops_test.go (added subtests to the list-ops exercise #2443)
• ./exercises/practice/matrix/matrix_test.go (added usage of subtests for exercise matrix #2447)
• ./exercises/practice/ocr-numbers/ocr_numbers_test.go (added usage of subtests and added descriptions for ocr-numbers #2455)
• ./exercises/practice/palindrome-products/palindrome_products_test.go (added usage of subtests for palindrome-products #2456)
• ./exercises/practice/pascals-triangle/pascals_triangle_test.go (added usage of subtests for exercise pascals-triangle #2457)
• ./exercises/practice/protein-translation/protein_translation_test.go (added usage of subtest for protein-translation #2460)
• ./exercises/practice/pythagorean-triplet/pythagorean_triplet_test.go (added usage of subtests for pythagorean-triplet #2461)
• ./exercises/practice/queen-attack/queen_attack_test.go (added usage of subtests for queen-attack #2463)
• ./exercises/practice/rotational-cipher/rotational_cipher_test.go (added usage of subtests for rotational cipher #2464)
• ./exercises/practice/series/ (added usage of subtests for series #2465)
• ./exercises/practice/spiral-matrix/spiral_matrix_test.go (added usage of subtests for spiral-matrix #2466)
• ./exercises/practice/strain/strain_test.go (usage of subtests for strain #2467)
• ./exercises/practice/tournament/tournament_test.go (added usage of subtests for tournament #2468)
• ./exercises/practice/triangle/triangle_test.go (added usage of subtests for triangle #2469)
• ./exercises/practice/twelve-days/twelve_days_test.go (added usage of subtests for twelve_days #2470)
• ./exercises/practice/two-fer/two_fer_test.go (added usage of subtests for two-fer #2471)
• ./exercises/practice/robot-simulator/

Might not be needed:

• ./exercises/practice/grade-school/grade_school_test.go (seems not to need)
• ./exercises/practice/hello-world/hello_world_test.go (seems not to need)
• ./exercises/practice/parallel-letter-frequency/parallel_letter_frequency_test.go (seems not to need)
• ./exercises/practice/react/react_test.go (seems not to need)
• ./exercises/practice/simple-linked-list/simple_linked_list_test.go (seems not to need)
• ./exercises/practice/zebra-puzzle/zebra_puzzle_test.go (seems not to need)
• ./exercises/practice/circular-buffer/circular_buffer_test.go (seems not to need)
• ./exercises/practice/diffie-hellman/diffie_hellman_test.go (not sure if needed)
• ./exercises/practice/error-handling/error_handling_test.go (seems not to need)
• ./exercises/practice/paasio/paasio_test.go (seems not to need)
• ./exercises/concept/animal-magic/animal_magic_test.go (seems not to need)
• ./exercises/practice/bank-account/bank_account_test.go (seems not to need)
• ./exercises/concept/deep-thought/deep_thought_test.go (exercise is WIP)
• ./exercises/concept/weather-forecast/weather_forecast_test.go (not needed, as the exercise is only about comments)
• ./exercises/practice/accumulate/accumulate_test.go (deprecated)
• ./exercises/practice/counter/counter_test.go (deprecated)
• ./exercises/practice/hexadecimal/hexadecimal_test.go (added usage of subtests for hexadecimal #2436-->(deprecated))
• ./exercises/practice/binary/binary_test.go (added subtests for exercise binary #2429) (deprecated)
• ./exercises/practice/octal/octal_test.go (deprecated)
• ./exercises/practice/trinary/trinary_test.go (deprecated)

[eklatzer]

@junedev or @andrerfcsantos
Could one of you check the exercises without a ✔️ and check if one of them needs usage of subtests, I am not sure about all of them, so double checking would be nice 😄 Thanks in advance
#2098 (comment)

[andrerfcsantos]

Left my analysis below. This was a quick analysis of each exercise. If there's an exercise for which I wrote that we maybe could add subtests, but when tackling you conclude that is not viable, feel free to say so.

I also noticed some exercises are missing generators. Adding generators is of course optional for this task, but some refactoring into subtests could be made in a way that makes writing a future generator easier. But since you are our expert at writing generators, feel free to also add generators and we'll size the PRs accordingly ;) But it's totally ok to do some of the refactoring into subtests now and leave the generator for later, especially because I feel the generators for some exercises will require quite some work.

---

• Robot Simulator

Seems robot_simulator_test.go might not need it but seems robot_simulator_step2_test.go and robot_simulator_step3_test.go need it, as each iteration of the loop seems to be a test?

• Grade School

grade_school_test.go might not need right now, but with a little refactoring maybe. For instance, it seems that TestAddMoreSameGrade and TestAddDifferentGrades are doing the same things, they take a list of students and add them. Looking at canonical-data.json, it seems these tests could be grouped by "property" and each one could be a subtest.

• Hello World

I think it's safe to skip this one - it only has one test and I don't think it'll ever change, so it's fine to be a regular test.

• Parallel Letter Frequency

The tests here were tailor-made for the track and there's only a single test for each function, so I think we can also skip this one.

• React

A similar situation to Grade School - there's some duplicated code between functions which could be grouped into single functions and maybe we could add a generator. Although I expect the creation of the generator and the refactoring to be significantly harder and require more thought than Grade School.

These could also be 2 separate tasks - 1 to refactor into subtests and another one to add the generator.

• Simple Linked List

There seems to also be some duplicated code in this one between functions, so adding subtests at least for some tests could be a good idea. The tests seem to be a list of instructions, maybe the subtest definition itself can contain a list of string containing the operation to do at each stage.

• Zebra puzzle

Only has one test currently and canonical data also only specify a single test, so it's fine to leave as it is. We could add a subtest, but it seems overkill for this case.

• Circular Buffer

Similar to Simple Linked List - the tests can be refactored into subtests in a similar way.

• Diffie Hellman

Looks like some functions are looping over tests, so maybe they are good candidates for subtests.

• Error Handling

Also some duplication in the code that maybe can be refactored into subtests.

• Paasio

Some duplication in code, the function testWrite also seems to iterate over a list of tests, but not using subtests, maybe a good candidate for them?

• Animal Magic

Safe to skip this one, tests are custom for this exercise and there isn't really code duplication.

• Bank Account

I also don't see an obvious way to refactor this into subtests.