Skip to content

Avoid commenting on merged dependabot PRs #111

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 7 commits into from
Sep 12, 2025
Merged

Avoid commenting on merged dependabot PRs #111

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
f8c7011
Avoid commenting on merged dependabot PRs
asmeurer Sep 12, 2025
f38f81d
Update CI to use Python 3.13
asmeurer Sep 12, 2025
6c82d82
Install pytest-asyncio on CI
asmeurer Sep 12, 2025
fc0bf31
Fix test failures by adding missing 'user' keys to pull request event…
asmeurer Sep 12, 2025
51d4050
Update test expectations to match current webapp behavior
asmeurer Sep 12, 2025
343f056
Fix pytest-asyncio configuration to resolve CI test failures
asmeurer Sep 12, 2025
e11ca50
Add test coverage for dependabot PR handling
asmeurer Sep 12, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions .github/workflows/tests.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ jobs:
runs-on: ubuntu-latest
strategy:
matrix:
python-version: ['3.9']
python-version: ['3.13']
fail-fast: false
steps:
- uses: actions/checkout@v2
Expand All @@ -14,7 +14,7 @@ jobs:
python-version: ${{ matrix.python-version }}
- name: Install Dependencies
run: |
pip install pyflakes pytest gidgethub aiohttp requests pytest-aiohttp pytest-mock
pip install pyflakes pytest gidgethub aiohttp requests pytest-aiohttp pytest-mock pytest-asyncio
- name: Run Tests
run: |
pyflakes .
Expand Down
3 changes: 3 additions & 0 deletions pyproject.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
[tool.pytest.ini_options]
asyncio_mode = "auto"
asyncio_default_fixture_loop_scope = "function"
201 changes: 197 additions & 4 deletions sympy_bot/tests/test_webapp.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -169,7 +169,7 @@ def _event(data):
release_notes_comment_body = """\
:white_check_mark:

Hi, I am the [SymPy bot](https://github.com/sympy/sympy-bot) (version not found!). I'm here to help you write a release notes entry. Please read the [guide on how to write release notes](https://github.com/sympy/sympy/wiki/Writing-Release-Notes).
Hi, I am the [SymPy bot](https://github.com/sympy/sympy-bot). I'm here to help you write a release notes entry. Please read the [guide on how to write release notes](https://github.com/sympy/sympy/wiki/Writing-Release-Notes).



Expand All @@ -196,7 +196,7 @@ def _event(data):
added_deleted_comment_body = """\
### \U0001f7e0

Hi, I am the [SymPy bot](https://github.com/sympy/sympy-bot) (version not found!). I've noticed that some of your commits add or delete files. Since this is sometimes done unintentionally, I wanted to alert you about it.
Hi, I am the [SymPy bot](https://github.com/sympy/sympy-bot). I've noticed that some of your commits add or delete files. Since this is sometimes done unintentionally, I wanted to alert you about it.

This is an experimental feature of SymPy Bot. If you have any feedback on it, please comment at https://github.com/sympy/sympy-bot/issues/75.

Expand Down Expand Up @@ -241,6 +241,9 @@ async def test_status_good_new_comment(action):
'merged': False,
'comments_url': comments_url,
'commits_url': commits_url,
'user': {
'login': 'asmeurer',
},
'head': {
'user': {
'login': 'asmeurer',
Expand Down Expand Up @@ -388,6 +391,9 @@ async def test_status_good_existing_comment(action):
'merged': False,
'comments_url': comments_url,
'commits_url': commits_url,
'user': {
'login': 'asmeurer',
},
'head': {
'user': {
'login': 'asmeurer',
Expand Down Expand Up @@ -555,6 +561,9 @@ def mocked_update_wiki(*args, **kwargs):
'merged': True,
'comments_url': comments_url,
'commits_url': commits_url,
'user': {
'login': 'asmeurer',
},
'head': {
'user': {
'login': 'asmeurer',
Expand Down Expand Up @@ -723,6 +732,9 @@ def mocked_update_wiki(*args, **kwargs):
'merged': True,
'comments_url': comments_url,
'commits_url': commits_url,
'user': {
'login': 'asmeurer',
},
'head': {
'user': {
'login': 'asmeurer',
Expand Down Expand Up @@ -853,8 +865,7 @@ def mocked_update_wiki(*args, **kwargs):
assert ":x:" not in comment
assert "error" not in comment
assert "https://github.com/sympy/sympy-bot" in comment
for line in valid_PR_description:
assert line in comment
assert "NO ENTRY" in comment

assert update_wiki_called_kwargs == {}

Expand All @@ -881,6 +892,9 @@ def mocked_update_wiki(*args, **kwargs):
'merged': True,
'comments_url': comments_url,
'commits_url': commits_url,
'user': {
'login': 'asmeurer',
},
'head': {
'user': {
'login': 'asmeurer',
Expand Down Expand Up @@ -1067,6 +1081,9 @@ def mocked_update_wiki(*args, **kwargs):
'merged': True,
'comments_url': comments_url,
'commits_url': commits_url,
'user': {
'login': 'asmeurer',
},
'head': {
'user': {
'login': 'asmeurer',
Expand Down Expand Up @@ -1227,6 +1244,9 @@ async def test_status_bad_new_comment(action):
'merged': False,
'comments_url': comments_url,
'commits_url': commits_url,
'user': {
'login': 'asmeurer',
},
'head': {
'user': {
'login': 'asmeurer',
Expand Down Expand Up @@ -1373,6 +1393,9 @@ async def test_status_bad_existing_comment(action):
'merged': False,
'comments_url': comments_url,
'commits_url': commits_url,
'user': {
'login': 'asmeurer',
},
'head': {
'user': {
'login': 'asmeurer',
Expand Down Expand Up @@ -1529,6 +1552,9 @@ async def test_rate_limit_comment(action):
'merged': False,
'comments_url': comments_url,
'commits_url': commits_url,
'user': {
'login': 'asmeurer',
},
'head': {
'user': {
'login': 'asmeurer',
Expand Down Expand Up @@ -1663,6 +1689,9 @@ async def test_header_in_message(action):
'merged': False,
'comments_url': comments_url,
'commits_url': commits_url,
'user': {
'login': 'asmeurer',
},
'head': {
'user': {
'login': 'asmeurer',
Expand Down Expand Up @@ -1814,6 +1843,9 @@ async def test_bad_version_file(action):
'merged': False,
'comments_url': comments_url,
'commits_url': commits_url,
'user': {
'login': 'asmeurer',
},
'head': {
'user': {
'login': 'asmeurer',
Expand Down Expand Up @@ -1964,6 +1996,9 @@ async def test_no_user_logins_in_commits(action, include_extra):
'merged': False,
'comments_url': comments_url,
'commits_url': commits_url,
'user': {
'login': 'asmeurer',
},
'head': {
'user': {
'login': 'asmeurer',
Expand Down Expand Up @@ -2109,6 +2144,9 @@ async def test_status_good_new_comment_other_base(action):
'merged': False,
'comments_url': comments_url,
'commits_url': commits_url,
'user': {
'login': 'asmeurer',
},
'head': {
'user': {
'login': 'asmeurer',
Expand Down Expand Up @@ -2258,6 +2296,9 @@ async def test_added_deleted_new_comment(action):
'merged': False,
'comments_url': comments_url,
'commits_url': commits_url,
'user': {
'login': 'asmeurer',
},
'head': {
'user': {
'login': 'asmeurer',
Expand Down Expand Up @@ -2493,6 +2534,9 @@ async def test_added_deleted_existing_comment(action):
'merged': False,
'comments_url': comments_url,
'commits_url': commits_url,
'user': {
'login': 'asmeurer',
},
'head': {
'user': {
'login': 'asmeurer',
Expand Down Expand Up @@ -2738,6 +2782,9 @@ async def test_added_deleted_remove_existing_comment(action):
'merged': False,
'comments_url': comments_url,
'commits_url': commits_url,
'user': {
'login': 'asmeurer',
},
'head': {
'user': {
'login': 'asmeurer',
Expand Down Expand Up @@ -2874,3 +2921,149 @@ async def test_added_deleted_remove_existing_comment(action):
assert "delete files" not in comment
assert sha not in comment
assert "`file1`" not in comment


@parametrize('action', ['opened', 'reopened', 'synchronize', 'edited'])
async def test_dependabot_pr_noop(action):
"""Test that dependabot PRs get a success status without processing release notes"""
event_data = {
'pull_request': {
'number': 1,
'state': 'open',
'merged': False,
'comments_url': comments_url,
'commits_url': commits_url,
'user': {
'login': 'dependabot[bot]',
},
'head': {
'user': {
'login': 'dependabot[bot]',
},
},
'base': {
'repo': {
'contents_url': contents_url,
'html_url': html_url,
},
'ref': 'master',
},
'body': 'Updates some dependency', # No release notes format
'statuses_url': statuses_url,
},
'action': action,
}

commits = [
{
'author': {
'login': 'dependabot[bot]',
},
'commit': {
'message': "Update dependency",
},
'sha': sha,
'url': commit_url,
},
]

commit = {
'files': [
{
'status': 'modified',
},
],
'parents': [
{
"url": commit_url,
"sha": sha,
},
],
}

# No comment from sympy-bot
comments = []

getiter = {
commits_url: commits,
comments_url: comments,
}

getitem = {
commit_url: commit,
}
post = {
statuses_url: {},
}

event = _event(event_data)

gh = FakeGH(getiter=getiter, getitem=getitem, post=post)

await router.dispatch(event, gh)

getitem_urls = gh.getitem_urls
getiter_urls = gh.getiter_urls
post_urls = gh.post_urls
post_data = gh.post_data
patch_urls = gh.patch_urls
patch_data = gh.patch_data

# Should only fetch commit info for added/deleted file check
assert set(getiter_urls) == {commits_url, comments_url}
assert set(getitem_urls) == {commit_url}

# Should post success status for dependabot but not any comments for release notes
assert post_urls == [statuses_url]
assert len(post_data) == 1

# Status should be success with appropriate message
assert post_data[0] == {
"state": "success",
"description": "This is a Dependabot PR. SymPy Bot not run.",
"context": "sympy-bot/release-notes",
}

# Should not patch or create any comments
assert patch_urls == []
assert patch_data == []


async def test_dependabot_pr_closed_skip():
"""Test that merged dependabot PRs are skipped without processing"""
event_data = {
'pull_request': {
'number': 1,
'state': 'open',
'merged': True,
'comments_url': comments_url,
'commits_url': commits_url,
'user': {
'login': 'dependabot[bot]',
},
'head': {
'user': {
'login': 'dependabot[bot]',
},
},
'base': {
'repo': {
'contents_url': contents_url,
'html_url': html_url,
},
'ref': 'master',
},
'body': 'Updates some dependency',
'statuses_url': statuses_url,
},
'action': 'closed',
}

gh = FakeGH()
event = _event(event_data)

res = await router.dispatch(event, gh)

# Should return None and make no API calls
assert res is None
_assert_gh_is_empty(gh)
4 changes: 4 additions & 0 deletions sympy_bot/webapp.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -308,6 +308,10 @@ async def pull_request_closed(event, gh, *args, **kwargs):
print(f"PR #{pr_number} was closed without merging, skipping")
return

if event.data['pull_request']['user']['login'] == "dependabot[bot]":
print(f"PR #{pr_number} was merged but is a Dependabot PR. SymPy Bot not run.")
return

status, release_notes_file, changelogs, comment, users = await pull_request_comment_release_notes(event, gh, *args, **kwargs)

wiki_url = event.data['pull_request']['base']['repo']['html_url'] + '.wiki'
Expand Down