unittest fails on the master branch

[nahcnuj]

I encountered an error when running unittest on the master branch of the repository right after cloning it. Below are the details of the error:

Steps to Reproduce:

1. Clone the repository.
2. Checkout the master branch.

   vscode ➜ /workspaces/todo-to-issue-action (master) $ git show
   commit c5294a65e603f820f52db936864ee68fa2dc47a8 (HEAD -> master, origin/master, origin/HEAD)
   Merge: 9733933 4417cd5
   Author: Alastair Mooney <[email protected]>
   Date:   Thu Jan 2 15:26:31 2025 +0000
   
       Merge pull request #255 from netpoe/syntax/move
    
       feat(syntax.json): Move language .move
   

3. Run the command: python -m unittest

Observed Behavior:
The following error is displayed:

vscode ➜ /workspaces/todo-to-issue-action (master) $ python -m unittest
.E..Success: no issues found in 1 source file
.GitHubClient.py:9: error: Need type annotation for "existing_issues" (hint: "existing_issues: list[<type>] = ...")  [var-annotated]
GitHubClient.py:10: error: Need type annotation for "milestones" (hint: "milestones: list[<type>] = ...")  [var-annotated]
Found 2 errors in 1 file (checked 1 source file)
x...............................x...............................
======================================================================
ERROR: test_line_numbering_with_deletions (tests.test_process_diff.IssueUrlInsertionTest.test_line_numbering_with_deletions)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/workspaces/todo-to-issue-action/tests/test_process_diff.py", line 84, in test_line_numbering_with_deletions
    self._setUp(['test_new_py.diff', 'test_edit_py.diff'])
  File "/workspaces/todo-to-issue-action/tests/test_process_diff.py", line 33, in _setUp
    subprocess.run(['patch', '-d', self.tempdir.name,
  File "/usr/local/lib/python3.12/subprocess.py", line 571, in run
    raise CalledProcessError(retcode, process.args,
subprocess.CalledProcessError: Command '['patch', '-d', '/tmp/tmp8ab527hn', '-i', '/workspaces/todo-to-issue-action/tests/test_edit_py.diff']' returned non-zero exit status 1.

----------------------------------------------------------------------
Ran 69 tests in 129.207s

FAILED (errors=1, expected failures=2)

Expected Behavior:
All tests should pass without any errors.

[alstr]

Thanks for the report. I noticed this issue when there was a line missing from the end of the diff file, but after adding it that did seem to be resolved. It looks like the issue is still happening in forked repos (such as yours, and here). @rgalonso might know why this still seems to occur in other repos, specifically with the test_closed.diff tests.

[rgalonso]

I can't replicate this issue when using the latest commit on master, c5294a65.

@nahcnuj, a few questions:

• are you certain you don't have any local modifications, especially within the tests subdirectory?
• are you running within the Dev Container or natively on your computer?
  • if the latter, what version of patch is installed?
    • run patch --version

Tip: As we debug this, you can limit the testing to just the one that's failing by running:

python -m unittest tests.test_process_diff.IssueUrlInsertionTest.test_line_numbering_with_deletions

@alstr, just FYI that the action run you linked to (for netpoe's fork) is one where the diff file hadn't been fixed yet to address this issue. (The log shows that its using commit 3ba6066.) So the fact that it shows a failure is unsurprising.

[rgalonso]

Also, a clarification regarding this statement:

this still seems to occur in other repos, specifically with the test_closed.diff tests.

This could in fact impact any test involving:

• test_new.diff
• test_new_py.diff
• test_edit_py.diff
• test_same_title_in_same_file.diff
• test_comment_suffix_after_source_line.diff

Specifically, if one of these files weren't formatted properly (for example, not ending with a blank line), then this issue would be triggered.

The patch processing isn't applied, currently, in any tests involving test_closed.diff, so that file isn't actually susceptible to the issue seen here. That said, it's good practice to ensure that the file is properly formatted in case future changes also start using this file in this manner.

[rgalonso]

@alstr, if it turns out that this is due to some subtle difference between versions of the system utility patch, we may want to use the patch Python module instead. That's probably a good idea regardless to avoid the system dependency and instead use a (likely more efficient and more deterministic) pure Python approach. That said, I'm not going to go implement that approach unnecessarily right now. I just wanted to record that idea here.

[nahcnuj]

Thank you for your quick response and for trying to replicate the issue.

• are you certain you don't have any local modifications, especially within the tests subdirectory?

Yes, I am certain there are no local modifications, especially within the tests subdirectory.

vscode ➜ /workspaces/todo-to-issue-action (master) $ git status 
On branch master
Your branch is up to date with 'origin/master'.

nothing to commit, working tree clean
vscode ➜ /workspaces/todo-to-issue-action (master) $ git show
commit c5294a65e603f820f52db936864ee68fa2dc47a8 (HEAD -> master, origin/master, origin/HEAD)
Merge: 9733933 4417cd5
Author: Alastair Mooney <[email protected]>
Date:   Thu Jan 2 15:26:31 2025 +0000

    Merge pull request #255 from netpoe/syntax/move
    
    feat(syntax.json): Move language .move

• are you running within the Dev Container or natively on your computer?

I am running within the Dev Container.

I appreciate the tip on limiting the test. I'll try running the specific test you mentioned and see if I can provide any more insights.

[alstr]

@rgalonso That makes sense, thanks.