-
-
Notifications
You must be signed in to change notification settings - Fork 649
Implement fuzztesting. #1139
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.
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
Implement fuzztesting. #1139
Changes from 5 commits
bc40efb
6c18da7
4a17780
75aa9fe
625cc0e
b078e92
f1f6718
c0ed1f9
998bd29
6b240b4
f060510
ab68f1a
d1d48ea
54f7fbe
12b5139
21283f7
c47b9fd
de81b25
5f79854
8330865
1ed4452
dd36f25
0a9068d
c5363db
ccc28c8
ef20adb
845e1c1
8755056
6637d8c
decec65
672f097
ed62759
49fa5f8
e1533ab
dcba769
5e85b0b
70312c5
751f105
4cb975a
47daeda
5e08107
de6bf2e
d908fcf
d3f2987
d3fd074
7d5c20d
03ecb0c
cf07402
e0459d4
54ce26a
deaf1d0
2236c73
8ada3b9
013537b
8ec969f
3839ef1
f1e2c56
2fabe70
56acd52
a2436f5
25b98f5
eb8c087
e27ce00
8d8e3fe
303f825
b57d2b2
8bb36c9
c79570c
09fd7c5
51a74b1
fdfc29d
663b3e5
c4eab67
133c89f
59007a9
2e4c7f8
ab32308
166f387
bf07032
e7b6329
a7862ac
958c84e
4076d72
88858ca
c65e4cb
7d33ba8
a48b578
27d468f
6d9f6a1
891baaf
4247342
c75ff58
b697d7a
a4d740b
cd56848
1640b3b
04c8f1d
b7d7f5d
fcc4927
240720b
df73958
e308775
c643ada
a6d5605
52a9b68
029db6e
de86aab
47d0feb
e62fed6
86184a7
5f7ad2d
2f78434
c400be4
7ea774e
fed517c
02b5e63
ea7e23d
2c59b86
f67d9ac
4edfb7b
2fcf6b9
db1a7a1
cefb150
7fb30f3
8444288
f79b403
6c2d0f0
6f15af2
9886459
c648ada
00b1f84
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,38 @@ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| from unittest.mock import MagicMock, patch | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| from django.conf import settings | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| from apps.slack.constants import ( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| OWASP_CONTRIBUTE_CHANNEL_ID, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| from apps.slack.events.member_joined_channel.contribute import contribute_handler | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| from hypothesis import given, strategies as st | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| class TestContributeEventHandler: | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| @given( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| events_enabled=st.booleans(), | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| project_count=st.integers(), | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| issue_count=st.integers(), | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| @patch("apps.owasp.models.project.Project.active_projects_count") | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| @patch("apps.github.models.issue.Issue.open_issues_count") | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| def test_handler_responses( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| self, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| mock_open_issues_count, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| mock_active_projects_count, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| events_enabled, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| project_count, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| issue_count, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ): | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| settings.SLACK_EVENTS_ENABLED = events_enabled | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| mock_active_projects_count.return_value = project_count | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| mock_open_issues_count.return_value = issue_count | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| mock_slack_event = {"user": "U123456", "channel": OWASP_CONTRIBUTE_CHANNEL_ID} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| mock_slack_client = MagicMock() | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| mock_slack_client.conversations_open.return_value = {"channel": {"id": "C123456"}} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| contribute_handler(event=mock_slack_event, client=mock_slack_client, ack=MagicMock()) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| assert True | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛠️ Refactor suggestion Add constraints to fuzz test parameters and strengthen assertions The current test doesn't place constraints on generated integers and uses a trivial assertion. Update the test with improved constraints and assertions: @given(
events_enabled=st.booleans(),
- project_count=st.integers(),
- issue_count=st.integers(),
+ project_count=st.integers(min_value=0),
+ issue_count=st.integers(min_value=0),
)
@patch("apps.owasp.models.project.Project.active_projects_count")
@patch("apps.github.models.issue.Issue.open_issues_count")
def test_handler_responses(
self,
mock_open_issues_count,
mock_active_projects_count,
events_enabled,
project_count,
issue_count,
):
settings.SLACK_EVENTS_ENABLED = events_enabled
mock_active_projects_count.return_value = project_count
mock_open_issues_count.return_value = issue_count
mock_slack_event = {"user": "U123456", "channel": OWASP_CONTRIBUTE_CHANNEL_ID}
mock_slack_client = MagicMock()
mock_slack_client.conversations_open.return_value = {"channel": {"id": "C123456"}}
contribute_handler(event=mock_slack_event, client=mock_slack_client, ack=MagicMock())
- assert True
+ # Verify the behavior based on whether events are enabled
+ if events_enabled:
+ mock_slack_client.conversations_open.assert_called_once_with(user="U123456")
+ mock_slack_client.chat_postMessage.assert_called()
+ # You could also verify that the message contains the project_count and issue_count
+ else:
+ mock_slack_client.conversations_open.assert_not_called()
+ mock_slack_client.chat_postMessage.assert_not_called()Using non-negative integers for counts makes the test more realistic, as negative counts wouldn't make sense in this context. 📝 Committable suggestion
Suggested change
🧰 Tools🪛 Ruff (0.8.2)34-34: Blank line contains whitespace Remove whitespace from blank line (W293) 37-37: Blank line contains whitespace Remove whitespace from blank line (W293)
coderabbitai[bot] marked this conversation as resolved.
Outdated
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,41 @@ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| from unittest.mock import MagicMock | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| from django.conf import settings | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| from hypothesis import given, strategies as st | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| from apps.slack.constants import OWASP_GSOC_CHANNEL_ID | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| from apps.slack.events.member_joined_channel.gsoc import gsoc_handler | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| class TestGsocEventHandler: | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| @given( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| channel_id=st.text(), | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| def test_check_gsoc_handler(self, channel_id): | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| gsoc_module = __import__( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| "apps.slack.events.member_joined_channel.gsoc", | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| fromlist=["gsoc_handler"], | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| check_gsoc_handler = getattr( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| gsoc_module, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| "check_gsoc_handler", | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| lambda x: x.get("channel") == OWASP_GSOC_CHANNEL_ID, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| check_gsoc_handler({"channel": channel_id}) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| assert True | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
coderabbitai[bot] marked this conversation as resolved.
Outdated
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛠️ Refactor suggestion Simplify the dynamic import approach in The current implementation uses a complex dynamic import approach with a fallback lambda. This makes the test harder to understand and may hide issues with the actual - def test_check_gsoc_handler(self, channel_id):
- gsoc_module = __import__(
- "apps.slack.events.member_joined_channel.gsoc",
- fromlist=["gsoc_handler"],
- )
- check_gsoc_handler = getattr(
- gsoc_module,
- "check_gsoc_handler",
- lambda x: x.get("channel") == OWASP_GSOC_CHANNEL_ID,
- )
-
- check_gsoc_handler({"channel": channel_id})
-
- assert True
+ def test_check_gsoc_handler(self, channel_id):
+ """Test that check_gsoc_handler correctly identifies the GSOC channel."""
+ from apps.slack.events.member_joined_channel.gsoc import check_gsoc_handler
+
+ result = check_gsoc_handler({"channel": channel_id})
+
+ # Verify the function returns True only for the GSOC channel
+ expected = (channel_id == OWASP_GSOC_CHANNEL_ID)
+ assert result == expectedThis approach directly imports the function, tests it with the generated channel_id, and verifies it returns the expected result based on whether the channel matches the GSOC channel ID. 📝 Committable suggestion
Suggested change
🧰 Tools🪛 Ruff (0.8.2)24-24: Blank line contains whitespace Remove whitespace from blank line (W293) 26-26: Blank line contains whitespace Remove whitespace from blank line (W293) |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
coderabbitai[bot] marked this conversation as resolved.
Outdated
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| @given( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| events_enabled=st.booleans(), | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| def test_handler_responses(self, events_enabled): | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| settings.SLACK_EVENTS_ENABLED = events_enabled | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| mock_slack_event = {"user": "U123456", "channel": OWASP_GSOC_CHANNEL_ID} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| mock_slack_client = MagicMock() | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| mock_slack_client.conversations_open.return_value = {"channel": {"id": "C123456"}} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| gsoc_handler(event=mock_slack_event, client=mock_slack_client, ack=MagicMock()) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| assert True | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛠️ Refactor suggestion Strengthen handler_responses test with mock verifications The current test executes the handler but doesn't verify that it behaves correctly based on the Update the test to verify the expected behavior: @given(
events_enabled=st.booleans(),
)
def test_handler_responses(self, events_enabled):
settings.SLACK_EVENTS_ENABLED = events_enabled
mock_slack_event = {"user": "U123456", "channel": OWASP_GSOC_CHANNEL_ID}
mock_slack_client = MagicMock()
mock_slack_client.conversations_open.return_value = {"channel": {"id": "C123456"}}
gsoc_handler(event=mock_slack_event, client=mock_slack_client, ack=MagicMock())
- assert True
+ # Verify that the Slack client was used appropriately based on events_enabled
+ if events_enabled:
+ mock_slack_client.conversations_open.assert_called_once_with(user="U123456")
+ mock_slack_client.chat_postMessage.assert_called()
+ else:
+ # If events are disabled, no Slack client operations should be performed
+ mock_slack_client.conversations_open.assert_not_called()
+ mock_slack_client.chat_postMessage.assert_not_called()📝 Committable suggestion
Suggested change
🧰 Tools🪛 Ruff (0.8.2)37-37: Blank line contains whitespace Remove whitespace from blank line (W293) 39-39: Blank line contains whitespace Remove whitespace from blank line (W293)
coderabbitai[bot] marked this conversation as resolved.
Outdated
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 💡 Verification agent ❓ Verification inconclusiveVerify handler behavior with concrete assertions Similar to the previous test method, this test only checks that the code runs without exceptions, but doesn't validate that the handler behaves as expected when events are enabled or disabled. def test_handler_responses(self, events_enabled):
settings.SLACK_EVENTS_ENABLED = events_enabled
mock_slack_event = {"user": "U123456", "channel": OWASP_GSOC_CHANNEL_ID}
mock_slack_client = MagicMock()
mock_slack_client.conversations_open.return_value = {"channel": {"id": "C123456"}}
gsoc_handler(event=mock_slack_event, client=mock_slack_client, ack=MagicMock())
- assert True
+ # When events are enabled, the client should attempt to open a conversation
+ if events_enabled:
+ mock_slack_client.conversations_open.assert_called_once_with(user=mock_slack_event["user"])
+ else:
+ mock_slack_client.conversations_open.assert_not_called()Additionally, fix the formatting issues highlighted by the static analyzer: 🏁 Script executed: #!/bin/bash
# Replace trailing whitespace and ensure a newline at end of file
sed -i 's/[[:space:]]*$//' backend/tests/slack/events/fuzz_tests/user_joind_channel/gsoc_test.py
echo '' >> backend/tests/slack/events/fuzz_tests/user_joind_channel/gsoc_test.pyLength of output: 108 Action Required: Add concrete assertions for Slack handler behavior The current test in
For example, consider replacing: gsoc_handler(event=mock_slack_event, client=mock_slack_client, ack=MagicMock())
- assert True
+ if events_enabled:
+ mock_slack_client.conversations_open.assert_called_once_with(user=mock_slack_event["user"])
+ else:
+ mock_slack_client.conversations_open.assert_not_called()Additionally, the formatting issues reported (trailing whitespace removal and ensuring a trailing newline) have been addressed by the static analyzer script. Please verify manually that these formatting fixes have been applied correctly to the file. 📝 Committable suggestion
Suggested change
🧰 Tools🪛 Ruff (0.8.2)37-37: Blank line contains whitespace Remove whitespace from blank line (W293) 39-39: Blank line contains whitespace Remove whitespace from blank line (W293) 41-41: Blank line contains whitespace Remove whitespace from blank line (W293) 41-41: No newline at end of file Add trailing newline (W292) |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
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.
🛠️ Refactor suggestion
Constrain strategy value ranges for page and hits_per_page.
Currently, the
pageandhits_per_pagestrategies allow any integer value, including negatives and zero, which are likely invalid inputs in the production code. The existing unit tests in this file (lines 107-110) show that validation is needed for these parameters.This change aligns with the validation being performed in the implementation and matches the constraints in the parametrized tests above.
📝 Committable suggestion