[JinJa2] Corrected the macro used for comments on the test file.

[BethanyG]

Per discussion on the forum
@ErikSchierboom @glennj - please feel free to edit/discuss/comment as you see fit.

Note: This will fail CI until all the regenerated test files are added into it.

This change, should we commit it and re-generate the test files, will show up for every practice exercise except:

• Robot Name
• Bank Account
• Hangman
• Simple Linke List
• Tree Building
• Ledger
• Paas/io
• Dot DSL

Which need a JinJa2 template, canonical-data.json, or both.

Partial sample re-generated test file:

import unittest

from word_count import (
    count_words,
)

# These tests are auto-generated with test data from:
# https://github.com/exercism/problem-specifications/tree/main/exercises/word-count/canonical-data.json
# File last updated on 2023-03-23

class WordCountTest(unittest.TestCase):
    def test_count_one_word(self):
        self.assertEqual(count_words("word"), {"word": 1})

    def test_count_one_of_each_word(self):
        self.assertEqual(count_words("one of each"), {"one": 1, "of": 1, "each": 1})

    def test_multiple_occurrences_of_a_word(self):
        self.assertEqual(
            count_words("one fish two fish red fish blue fish"),
            {"one": 1, "fish": 4, "two": 1, "red": 1, "blue": 1},
        )

    def test_handles_cramped_lists(self):
        self.assertEqual(count_words("one,two,three"), {"one": 1, "two": 1, "three": 1})

    def test_handles_expanded_lists(self):
        self.assertEqual(
            count_words("one,\ntwo,\nthree"), {"one": 1, "two": 1, "three": 1}
        )

    def test_ignore_punctuation(self):
        self.assertEqual(
            count_words("car: carpet as java: javascript!!&@$%^&"),
            {"car": 1, "carpet": 1, "as": 1, "java": 1, "javascript": 1},
        )

        ...

[glennj]

That certainly resolves the uncertainty.

One thought: what if the canonical-data changes and then the linked data is not the data that generated the tests. Perhaps add the datetime?

[ErikSchierboom]

Nice!

[BethanyG]

@glennj - updated generate_tests.py, so datetime.utcnow() is available globally for the templates, and added a line to the macros. The example is updated above. Is that what you had in mind? 😄 LMK -- and I will regenerate the test files & and add them here. 🌈

[ee7]

@ErikSchierboom For a PR that makes a trivial change to a large number of exercises, but will never change the pass/fail status of a previously submitted solution, can we opt-out of re-running the tests for every submitted solution?

I asked this on the latest call, but Jeremy didn't see the question. @BethanyG wanted to know too.

I remember that this was discussed a long time ago, but I don't know if the opt-out was implemented - I've never done it.

[ErikSchierboom]

No, there is no such thing. I'm not sure how we would implement that. Via a special marker in the commit message?

[ee7]

Via a special marker in the commit message?

That's the best way I can think of.

Do you think it's worth implementing? I do. It seems like it can avoid creating gigantic backlogs in the test runners.

[BethanyG]

This would update 114 exercises, if I am doing the math correctly.
Sounds like maybe we want to pause merging this, if only to experiment a little with flagging?

If you want to run a flagging experiment, you can regenerate all practice test files by running

./bin generate_tests.py at the command line from the repo root on a branch.

./bin generate_test.py <exercise-slug> will do a single exercise.

[ErikSchierboom]

Do you think it's worth implementing? I do. It seems like it can avoid creating gigantic backlogs in the test runners.
Yeah, I think it's worth it.

This would update 114 exercises, if I am doing the math correctly.
Sounds like maybe we want to pause merging this, if only to experiment a little with flagging?

Yeah, we probably want to postpone the merging of the changes a bit. Especially on a popular track like Python there will be a lot of churn.

[ee7]

Yeah, I think it's worth it.

Cool. I've created exercism/exercism#6678.

[ee7]

Via a special marker in the commit message?

Another option: via a special label on the PR. But let's discuss in exercism/exercism#6678.

[iHiD]

FYI, We're working on this (and relatively close to a solution) but it's quite complex, and the consequences of getting it wrong aren't good for us, so we're just making sure all our 'i's are dotted and 't's are crossed :)

[BethanyG]

but it's quite complex, and the consequences of getting it wrong aren't good for us, so we're just making sure all our 'i's are dotted and 't's are crossed :)

And I very much appreciate that and the work that is going into it! 💙

Getting the escape sorted so that it works well for the platform will be great - there are a few potential changes on the horizon for Python that would trigger mass re-testing, and shouldn't:

• Finally removing all those if name == main footers
• Swapping unittest syntax for pytest syntax in test files
• Adding actual, expected ("got", "want") messaging for practice test cases.
• Adding markers to help in ordering test cases for practice exercises

..and more that I am probably forgetting.

[BethanyG]

@ErikSchierboom @iHiD - checking in on this. Angel and I are in the process of cleaning up footer macros in practice exercises (which will trigger re-testing without a flag), as well as working our way to changing some syntax and making other adjustments. See #3419 for an example.

Any ETA? 🙂

[ErikSchierboom]

No, sorry :( We're incredibly busy with Insiders

[iHiD]

We did mention this yesterday though when going through open PRs actually. I think the work is basically done, we just need to deploy it and not blow everything up when we do so, which involves having the brainspace to think that through. Which, as Erik says, is something we're struggling for. I'm off next week, but we could commit to doing it the week after.

[BethanyG]

Ugh. Note to self: utcnow is on the depreciation list for Python 3.12!! It will be a while until that happens, but it will bite me in the arse, so documenting it here. When this gets toward merge, I should test datetime.now(datetime.UTC) instead.

[BethanyG]

Noting here that PRs #3425 -- #3435 are work to remove the old and outdated

if __name__ == "__main__":
    unittest.main()

And are on hold until the flag is resolved.

[ErikSchierboom]

@BethanyG We're getting closer to fixing the problem!

[BethanyG]

Note: The way the CI is currently set up, all tests are regenerated and then compared to the ones generated from the PR branch. But with datetime, that is always going to fail! 😂 So, falling back to date, since even hour increment might fail in some circumstances.

Longer term, if time is needed, I will think through a diff skip of some sort.