Markdown exercise: additional test cases

Programming Python
1

Hi. I’d like to point out that current tests cases allow code that converts only the first occurrence of any bold or italic text in a line. Here’s a couple of test cases that could be added. Thanks!

def test_with_a_little_bit_of_everything_multiple_bold(self):
    self.assertEqual(
       parse("# Header!\n* __Bold Item__ __Another Bold Item__\n* _Italic Item_"),
       "<h1>Header!</h1><ul><li><strong>Bold Item</strong> <strong>Another Bold Item</strong></li><li><em>Italic Item</em></li></ul>",
    )

def test_with_a_little_bit_of_everything_multiple_italic(self):
    self.assertEqual(
        parse("# Header!\n* __Bold Item__ _Italic Between Bold_ __Another Bold Item__\n* _Italic Item_"),
        "<h1>Header!</h1><ul><li><strong>Bold Item</strong> <em>Italic Between Bold</em> <strong>Another Bold Item</strong></li><li><em>Italic Item</em></li></ul>",
    )
2

Thanks for the suggestion.

I’m going to link to Suggesting Exercise Improvements as it’s at the core of what we decide here.

I’m a tentative :+1: on this as an improvement. I think it’s easy to make this mistake (of only replacing the first instance) - it’s definitely a mistake I made early in my Ruby days, using sub instead of gsub. So because I think this can enhance learning, I see value in adding it. Additionally, I think if it breaks people’s solutions, then it’s because they were incorrect in the first place.

I suggest leaving this discussion open until Wednesday and if there’s no objections by then, @ukolka, you could open a PR then if you like?

3

No objections to the test.

A minor nit on the test language, though. Each bullet line is one item. Multiple bold sections in one bullet point are not multiple items. The suggested PR suggests the multiple bold sections of a single bullet point are multiple items.

Additionally, should this include multiple italicized sections on one line (assuming that doesn’t already exist in the tests)?

4

These seem OK, and I am a tentative :+1: .

But we may want to have a larger discussion on how far we go/how nested is too nested. Another consideration is weather or not the current code in the stub passes these tests. Since this is a refactoring exercise, we probably shouldn’t be introducing tests that the original code doesn’t pass. I’m not in front of a computer where I can run those tests, but will check later today my time (PST).

I do object to adding this to the Python track only. I think this is a discussion that should be had for all tracks, via problem specifications.

This isn’t a Python - specific issue, and maintaining these tests as Python-specific seems like the wrong way to go about it, since any track that implements this exercise likely has the same problems with student solutions.

Additionally, if this were to be added to the Python track, it would need to be via an additional_tests.json file (an example here), and not as an edit to the test file.

For Python, we auto-generate practice exercise test files using a template that sucks in JSON from problem specification json + additional_test json.

5

Just ran the stub against the proposed tests. The student is asked to refactor this code and make it … better … while still passing all the tests.

The stub code fails:


    def test_with_a_little_bit_of_everything_multiple_bold(self):
        self.assertEqual(
           parse("# Header!\n* __Bold Item__ __Another Bold Item__\n* _Italic Item_"),
           "<h1>Header!</h1><ul><li><strong>Bold Item</strong> <strong>Another Bold Item</strong></li><li><em>Italic Item</em></li></ul>",
        )

and passes:


    def test_with_a_little_bit_of_everything_multiple_italic(self):
        self.assertEqual(
            parse("# Header!\n* __Bold Item__ _Italic Between Bold_ __Another Bold Item__\n* _Italic Item_"),
            "<h1>Header!</h1><ul><li><strong>Bold Item</strong> <em>Italic Between Bold</em> <strong>Another Bold Item</strong></li><li><em>Italic Item</em></li></ul>",
        )

So I think only the test_with_a_little_bit_of_everything_multiple_italic test is a candidate for adding to the test cases at this time, unless we’d like to consider re-writing the stub code for any track that implements this exercise.

Conversely, we can propose a different second test that gets at a similar issue and which the stub code passes.