GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — Add test case for changed lined numbers in use-walrus-if by drdavella · Pull Request #336 · pixee/codemodder-python

Conversation

@drdavella

@drdavella drdavella commented Mar 6, 2024

Copy link
Copy Markdown
Contributor

Overview

Add (failing) test case for computed line numbers in use-walrus-if

Details

  • We need to make an update so that these change positions are computed properly

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
@drdavella
drdavella requested review from andrecsilva and clavedeluna as code owners March 6, 2024 20:49
@drdavella drdavella mentioned this pull request Mar 6, 2024
Properly compute change locations for use-walrus-if #337
Open
@drdavella

drdavella commented Mar 6, 2024

Copy link
Copy Markdown
Contributor Author

Bumped the unit test timeout since we keep hitting on this for the Python 3.12 tests. See #338.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@drdavella
drdavella enabled auto-merge March 6, 2024 21:27
andrecsilva
andrecsilva reviewed Mar 7, 2024
View reviewed changes

@andrecsilva andrecsilva left a comment

Copy link
Copy Markdown
Contributor

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure what is the problem here. The current behavior seems to be correct as it reports the node positions of the ifs in the original code.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
Comment thread tests/codemods/test_walrus_if.py Outdated
self.run_and_assert(tmpdir, input_code, expected_output)

@pytest.mark.xfail(reason="Need to recompute line numbers to reflect removed lines")
def test_multiple_warlus_changes(self, tmpdir):

@andrecsilva andrecsilva Mar 7, 2024

Copy link
Copy Markdown
Contributor

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

warlus -> walrus

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
@drdavella

drdavella commented Mar 7, 2024

Copy link
Copy Markdown
Contributor Author

@andrecsilva the line locations here affect the position of the comments in CodeTF. In general we frame our comments in terms of the fix rather than the problem and we want the comments to appear on the right side of the split diff, which means they need to be associated with new code positions.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
drdavella added 2 commits March 8, 2024 09:48
@drdavella
Add test case for changed lined numbers in use-walrus-if
21e741f
@drdavella
Bump unit test timeout to 25 mins
d6ac755
@drdavella
drdavella force-pushed the add-walrus-test-case branch from 7f2ae59 to d6ac755 Compare March 8, 2024 14:48
@drdavella
drdavella requested a review from andrecsilva March 8, 2024 14:48
@sonarqubecloud

sonarqubecloud Bot commented Mar 8, 2024

Copy link
Copy Markdown

Quality Gate Passed Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
No data about Coverage
0.0% Duplication on New Code

See analysis details on SonarCloud
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
clavedeluna
clavedeluna approved these changes Mar 8, 2024
View reviewed changes
@drdavella
drdavella added this pull request to the merge queue Mar 8, 2024
Merged via the queue into main with commit 576f621 Mar 8, 2024
@drdavella
drdavella deleted the add-walrus-test-case branch March 8, 2024 15:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@andrecsilva andrecsilva Awaiting requested review from andrecsilva
1 more reviewer
@clavedeluna clavedeluna clavedeluna approved these changes
Reviewers whose approvals may not affect merge requirements

Uh oh!

There was an error while loading. Please reload this page.

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Uh oh!

There was an error while loading. Please reload this page.

3 participants

@drdavella @andrecsilva @clavedeluna
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.