- Notifications
You must be signed in to change notification settings - Fork11.2k
test: Cover scrapy.extensions.memusage.MemoryUsage#7004
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
base:master
Are you sure you want to change the base?
Uh oh!
There was an error while loading.Please reload this page.
Conversation
wRAR commentedAug 11, 2025
This is a good start but I would remove all tests that check internals of the extension and add tests that run a crawl and check that it finishes with proper log messages and finish reason. |
Disables correctly when turned off or when the resource module is missingLogs and updates stats when hitting warning or limit thresholdsSends email alerts for both warnings and hard limitsHandles normal usage without false alarms and tracks peak usageCleans up all running tasks when the spider closesbasic CrawlerRunner and validates settings validation
codecovbot commentedAug 11, 2025 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
❌ 8 Tests Failed:
View the top 3 failed test(s) by shortest run time
To view more test analytics, go to theTest Analytics Dashboard |
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
…h @deferred_f_from_coro_f, used use maybe_deferred_to_future()
tests/test_extension_memusage.py Outdated
| @mock.patch.object(MemoryUsage, "get_virtual_size") | ||
| def test_email_notification_on_limit(self, mock_get_size): | ||
| """Test that email notification is sent when limit is exceeded.""" |
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.
Can you explain how does this test that?
tests/test_extension_memusage.py Outdated
| assert size > 0 | ||
| def test_warning_only_triggered_once(self): | ||
| """Test that warning is only triggered once even if threshold is exceeded multiple times.""" |
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.
Can you explain how does this test that?
tests/test_extension_memusage.py Outdated
| # Initially not warned | ||
| assert not extension.warned | ||
| # Simulate warning triggered | ||
| extension.warned = True | ||
| # Should remain warned | ||
| assert extension.warned |
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.
Can you explain what is tested here?
wRAR commentedAug 13, 2025
Please make sure the tests work locally before publishing the changes. |
Uh oh!
There was an error while loading.Please reload this page.
fixes#7002