[IsaacG]

No description provided.

[IsaacG]

@BethanyG How's this look for issue #2786 ?

[BethanyG]

@IsaacG - The example solution as provided is failing 7 of the tests as they've been generated by the JinJa2 template -- which is why the CI is failing. Looks like the tests are expecting a return value, but getting None instead.

So at a glance, it appears there is something up with the context of the helper and the code under test. Return values from the code under test are not being returned/passed to pytest for validation.

I don't have time to dig further at the moment, but you may want to try putting conditional logic in the template directly to call functions twice for the immutability tests. The helper class appears as if it ought to work just fine ... but it's not.

[BethanyG]

Actually @IsaacG, that last message was a massive brain fart on my part. I was pointed at the wrong repo! 😱

But this is failing in the test runner. Something about this is fouling up the code that navigates the AST and puts tests in order, so I will need to dig into that.

[IsaacG]

Yeah. I saw that after I created the PR. Yet when I run the test runner locally (outside Docker), it's perfectly happy. If the template/helper approach seems reasonable, I can try and dig into those. Alternatively, I can try to solve this without a helper class.

[BethanyG]

The quickest and easiest way would likely be to omit the helper class and do conditional logic in the test case selection, with a macro that treated immutability cases as "special", and added the extra function calls to catch if scores changed.

Something roughly along the lines of:

if its an immutability case, then poop out the following:

    def test_latest_score_after_personal_top_scores(self):
        scores = [70, 50, 20, 30]
        expected = 30
        temp_obj = HighScores(scores)
        temp_obj.personal_top_three()
        self.assertEqual(HighScores(temp_obj.scores).latest(), expected)

The website uses the Docker container for tests, so its the Docker version that needs to work.

The test runner sorting ceremony is ..brittle, and eventually needs to be replaced. It's intended to keep the tests in the order they're defined in the test file, but it's "solving" a problem that we may no longer need to have solved. However, I am not ready to jump into refactoring the test runner today....especially when it is only breaking for this one exercise.

[IsaacG]

Sure! That seems a bit ... messier of an approach, but I can give it a spin. I'll update the PR when I got it working.

[IsaacG]

This should handle all the tests in the canonical data, including the ones unsynced at the parent commit.

[IsaacG]

... I'm not sure what's up with that Docker test runner 🤷

[BethanyG]

Sure! That seems a bit ... messier of an approach, but I can give it a spin. I'll update the PR when I got it working.

Yup. It is indeed messy. And you are welcome to make it more elegant....as long as it passes CI and runs in Docker correctly. 😉

... I'm not sure what's up with that Docker test runner 🤷

Yup. Me either, really. With the day I've had, I am ... uninterested in getting into it. But when I get a block of time free, I am going to kill it with fire....er.....refactor the heck out of it.

Thanks for submitting the updates! I'll take a look.

[BethanyG]

Looks good...and it fails where it's supposed to when I call self.scores.sort(reverse=True) instead of using sorted(). Thanks for your work on this!

[BethanyG]

...and if you are feeling particularly masochistic, you can now submit a PR to add a additional_tests.json with a test or two to check that HighScores(scores) stashes scores in a property.....or not. This was fairly painful.

[IsaacG]

Handling a HighScores.score attribute/property ought to be relatively simple ... so long as you're fine with the template being messy. Though differentiating latestAfter from scoresAfter might get tricksy.

[IsaacG]

See PR #3016 for the scores. I think I made it slightly cleaner ... but added a macro.

[BethanyG]

That looks good ...BUT. We want the additional cases in a additional_tests.json file. Like this one for Clock.

Those then get concatenated with the canonical data and run through the template. We shouldn't do a complete end-runaround via macro. As much as we'd like to. 😉

[BethanyG]

Then again, the way you've done it does call HighScores.scores, and is effectively the same test cases.... 🤔

[IsaacG]

I'm not following. All these tests are implementing test cases from the canonical data. Aren't additional tests for tests which are not pulled from the canonical data?