exercism / python-representer

GNU Affero General Public License v3.0
5 stars 9 forks source link

Add Additional Smoke Tests for Newer Python Features from `3.9`, `3.10`, `3.11`, and `3.12` #50

Open BethanyG opened 7 months ago

BethanyG commented 7 months ago

We are largely "safe", now that we've stopped using astor, but we should for completeness add additional smoke tests for features such as:

brocla commented 7 months ago

I'd like to contribute some tests. Perhaps something like this,

    def test_walrus_assignment(self):
        before = """\
                (x := 1)
                (y := 0, 1)
                (z := (0, 1))
                """
        expect = """\
                (placeholder_0 := 1)
                ((placeholder_1 := 0), 1)
                (placeholder_2 := (0, 1))
                """
        self.assertCodeEqual(before, expect)

Would that be helpful?

I can't create a PR yet because the python-representer branches on github are still using astor, not ast.unparse.

BethanyG commented 7 months ago

@brocla -- I'd welcome some tests! Two caveats:

  1. We need to wait for Erik to merge PR #45 and re-run representations on the site. This could take a while, since we need to do that when it won't overload things/cause slowdowns. While we could branch from my branch -- I just don't want to do that. 😬
  2. The tests are a bit fiddly, since we're doing "golden tests". We're essentially recording results, and then comparing new runs to the results that were pre-recorded.

An example test for structural pattern matching can be found here in my fork that the current open PR is based off of.

So the steps would be (updated post-PR 45 merge):

  1. Write some code that uses the feature/function/thing (prefer that this code be similar/analogous to what a student solution might look like)
  2. Create a 'solutions' file, like this example_print_removal.py file.
  3. Run that code through the representer (in docker) to get the 4 output files.
  4. Make sure that those output files are what is expected for the code you wrote.
  5. If everything looks good manually, create a directory for the test under the test/ directory. All of these tests would follow the example naming convention e.g, example-walrus-normalization
  6. Add the 4 output files and the code. Here is an existing one, to follow.
  7. Create the .meta/ directory, and the config.json file inside it. here is one you can copy. Obviously, your name as author, and the name of the file as the 'solution'.
brocla commented 7 months ago

Hi Bethany,

That sounds like a good plan. Thanks for the detail. It will be very helpful. While waiting for PR#45, I'll start gathering up some code snippets for the solutions file and composing some unit tests.

Thanks Kevin

On Tue, Mar 5, 2024 at 5:08 PM BethanyG @.***> wrote:

@brocla https://github.com/brocla -- I'd welcome some tests! Two caveats:

  1. We need to wait for Erik to merge PR #45 https://github.com/exercism/python-representer/pull/45 and re-run representations on the site. This could take a while, since we need to do that when it won't overload things/cause slowdowns. While we could branch from my branch -- I just don't want to do that. 😬
  2. The tests are a bit fiddly, since we're doing "golden tests". We're essentially recording results, and then comparing new runs to the results that were pre-recorded.

An example test for structural pattern matching can be found here https://github.com/BethanyG/python-representer/blob/fix-representations/test/example-structural-pattern-matching/example_structural_pattern_matching.py in my fork that the current open PR is based off of.

So the steps would be:

  1. Write some code that uses the feature/function/thing (prefer that this code be similar/analogous to what a student solution https://exercism.org/tracks/python/exercises/pig-latin/solutions/cmccandless might look like)
  2. Create a 'solutions' file, like this example_print_removal.py file https://github.com/BethanyG/python-representer/blob/fix-representations/test/example-print-removal/example_print_removal.py .
  3. Run that code through the representer (in docker https://github.com/exercism/python-representer?tab=readme-ov-file#creating-a-representation) to get the 4 output files.
  4. Make sure that those output files are what is expected for the code you wrote.
  5. If everything looks good manually, create a directory for the test under the test/ https://github.com/BethanyG/python-representer/tree/fix-representations/test directory. All of these tests would follow the example naming convention e.g, example-walrus-normalization
  6. Add the 4 output files and the code. Here is an existing one https://github.com/BethanyG/python-representer/tree/fix-representations/test/example-docstring-removal, to follow.
  7. Create the .meta/ directory, and the config.json file inside it. here https://github.com/BethanyG/python-representer/blob/fix-representations/test/example-docstring-removal/.meta/config.json is one you can copy. Obviously, your name as author, and the name of the file as the 'solution'.

— Reply to this email directly, view it on GitHub https://github.com/exercism/python-representer/issues/50#issuecomment-1979844574, or unsubscribe https://github.com/notifications/unsubscribe-auth/ACBULKWUSZ3CHVBXNZCQSJTYWZM7XAVCNFSM6AAAAABEHE4Z5KVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTSNZZHA2DINJXGQ . You are receiving this because you were mentioned.Message ID: @.***>

BethanyG commented 7 months ago

Hey Kevin,

Looks like Erik merged PR 45 this morning, and the representation re-runs are in process. I think you can go ahead and fork the repo and make a branch for additional tests, since astor is no longer part of main. 😄

Feel free to ping me here, on our Discord server, or on our forum, should you have any questions or issues. My username is the same in all of them.

-Bethany

brocla commented 7 months ago

I posted PR#51, but it was immediately closed. I was pointed to the community forum. I can post something there if that helps.

Kevin

kotp commented 7 months ago

I posted #51 , but it was immediately closed. I was pointed to the community forum. I can post something there if that helps.

Kevin

That is the reason that the message says to do that. Linking to the forum post from there and likely mentioning this issue will help keep the information well connected.

I quoted (with an edit that makes the PR link work) to gain the linking mechanism so others can easily click to see what you are referencing.

BethanyG commented 7 months ago

@kotp -- we've already pre-agreed (as you can see by the discussion). I am also replying. Posting to the forum is not needed in this curumstance.

BethanyG commented 7 months ago

@brocla -- posting to the forum is not needed, since we've already discussed and agreed on the work.

The auto-close and notice are an Exercism-wide policy designed to discourage "drive by" commits by those who aren't clear on how to contribute, or who need to clarify what the work is, and weather or not it is needed/wanted by maintainers.

kotp commented 7 months ago

@kotp -- we've already pre-agreed (as you can see by the discussion). I am also replying. Posting to the forum is not needed in this curumstance.

The reply was mostly for the benefit of having a working link to the PR that was mentioned, and in general the reading that is presented being helpful to indicate. If other activity contradicts that, then it is obvious it is not a wanted step. However, then, instead of linking to the forum, linking to the thing that contra-indicates is helpful.

I think we are on the same page here, of course.