aboutsummaryrefslogtreecommitdiffstats
Commit message (Collapse)AuthorAgeLines
...
| | | | | | | | * | | Scheduler: remove create_task functionGravatar MarkKoz2020-02-15-15/+7
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | It's redundant because the done callback only takes a single line to add and can be added in schedule_task(). * Use Task as the type hint rather than Future for _suppress_cancelled_error()
| | | | | | | | * | | Scheduler: fix #754 - only suppress CancelledErrorGravatar MarkKoz2020-02-15-6/+7
| | | | | |_|_|/ / / | | | | |/| | | | |
| | | | | | | | * | Use MagicMock as return value for _get_diff mockGravatar Sebastiaan Zeeff2020-02-24-2/+2
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | The `_get_diff` method of TestSyncer class is mocked using an AsyncMock object. By default, when an AsyncMock object is called **and awaited**, it returns a child mock of the same time (another AsyncMock) according to the "the child is a like the parent" principle. This means that the _get_diff method will return an AsyncMock unless a different return_value is explicitly provided. Because of that "child is like parent" behavior, this will happen in lines 194-196 of bot.cogs.sync.syncers (annotations added by me): ``` // `diff` will be a child AsyncMock as "child is like parent" diff = await self._get_diff(guild) // `diff._asdict` will be an AsyncMock as "child is like parent" and, // after being called, it will return an unawaited coroutine object // we assign the name `diff_dict`: diff_dict = diff._asdict() // `diff_dict` is still an unawaited coroutine object meaning that it // doesn't have an `items()` method: totals = {k: len(v) for k, v in diff_dict.items() if v is not None} ``` Original, unannotated: https://github.com/python-discord/bot/blob/c81a4d401ea434e98b0a1ece51d3d10f1a3ad226/bot/cogs/sync/syncers.py#L194-L196 This will lead to the following exception when running the tests: ```py ====================================================================== ERROR: test_sync_confirmation_context_redirect (tests.bot.cogs.sync.test_base.SyncerSyncTests) (ctx=None, author=<MockMember name='mock.user' spec_set='Member' id='140583452034864'>, message=None) If ctx is given, a new message should be sent and author should be ctx's author. ---------------------------------------------------------------------- Traceback (most recent call last): File "/home/sebastiaan/pydis/repositories/bot/tests/bot/cogs/sync/test_base.py", line 348, in test_sync_confirmation_context_redirect await self.syncer.sync(guild, ctx) File "/home/sebastiaan/pydis/repositories/bot/bot/cogs/sync/syncers.py", line 196, in sync totals = {k: len(v) for k, v in diff_dict.items() if v is not None} AttributeError: 'coroutine' object has no attribute 'items' ``` The solution is to assign an explicit return value so the parent mock doesn't "guess" and return an object of its own type. I previously did that by providing a specific `_Diff` object as the return value, but I should have gone with a `MagicMock` to signify that it's not an important return value; it's just something that needs to support/mimic the API we use on it. So that's what this commit adds.
| | | | | | | | * | Sync tests: use autospec instead of MockSyncerGravatar MarkKoz2020-02-23-18/+4
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Autospec supports using AsyncMocks in 3.8 so there's no need to rely on a subclass of CustomMockMixin for the async mocks.
| | | | | | | | * | Migrate syncers test suite to Python 3.8Gravatar Sebastiaan Zeeff2020-02-24-75/+34
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | The test suite for the new role/member syncers used the "old"-style test suite with the helpers implemented for Python 3.7. I have migrated it to use the new Python 3.8 asyncio test helpers.
| | | | | | | | * | Use realistic mixin implementationGravatar Sebastiaan Zeeff2020-02-24-1/+1
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Instead of using the mixin class bare, I've now included into a class tha subclasses unittest.TestCase as that's how it's going to be used "in the wild".
| | | | | | | | * | Merge branch 'master' into python38-migrationGravatar Sebastiaan Zeeff2020-02-24-541/+1658
| | | | | | | | |\ \ | | |_|_|_|_|_|_|/ / | |/| | | | | | | |
| | | | | | | | * | Update Dockerfile to use python:3.8-slimGravatar Sebastiaan Zeeff2020-02-24-1/+1
| | | | | | | | | |
| | | | | | | | * | Make Azure CI use Python 3.8 and Ubuntu 18.04Gravatar Sebastiaan Zeeff2020-02-24-2/+2
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Since the bot is now using Python 3.8 and some of our dependencies have Python-version specfic dependencies, it's important that the CI, which installs from our Pipfile, uses the same version of Python.
| | | | | | | | * | Fix test_time test and ensure coverageGravatar Sebastiaan Zeeff2020-02-23-4/+4
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | One of the test_time methods did not actually assert the exception message it was trying to detect as the assertion statement was contained within the context manager handling the exception. I've moved it out of the context so it actually runs. I've also added a few `praga: no cover` comments for parts that were artifically lowering coverage of the test suite.
| | | | | | | | * | Use unittest's AsyncMock instead of our AsyncMockGravatar Sebastiaan Zeeff2020-02-23-213/+103
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Python 3.8 introduced an `unittest.mock.AsyncMock` class that can be used to mock coroutines and other types of asynchronous operations like async iterators and async context managers. As we were using our custom, but limited, AsyncMock, I have replaced our mock with unittest's AsyncMock. Since Python 3.8 also introduces a different way of automatically detecting which attributes should be mocked with an AsyncMock, I've changed our CustomMockMixin to use this new method as well. Together with a couple other small changes, this means that our Custom Mocks now use a lazy method of detecting coroutine attributes, which significantly speeds up the test suite.
| | | | | | | | * | Remove lingering pytest test_time.py fileGravatar Sebastiaan Zeeff2020-02-23-62/+0
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | I forgot to remove one pytest test file during the migration from pytest to unittest. Since we have sinced added a unittest version of the same file, I've now removed the lingering pytest file.
| | | | | | | | * | Use IsolatedAsyncioTestCase instead of async_testGravatar Sebastiaan Zeeff2020-02-23-70/+13
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Since we upgraded to Python 3.8, we can now use the new IsolatedAsyncioTestCase test class to use coroutine-based test methods instead of our own, custom async_test decorator. I have changed the base class for all of our test classes that use coroutine-based test methods and removed the now obsolete decorator from our helpers.
| | | | | | | | * | Use mixin-composition not inheritance for LoggingTestCaseGravatar Sebastiaan Zeeff2020-02-23-16/+14
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | We used inheritence to add additional logging assertion methods to unittest's TestCase class. However, with the introduction of the new IsolatedAsyncioTestCase this extension strategy means we'd have to create multiple child classes to be able to use the extended functionality in all of the TestCase variants. Since that leads to undesirable code reuse and an inheritance relationship is not at all needed, I've switched to a mixin-composition based approach that allows the user to extend the functionality of any TestCase variant with a mixin where needed.
| | | | | | | | * | Update to Python 3.8 and discord.py 1.3.2Gravatar Sebastiaan Zeeff2020-02-23-79/+37
| | | | |_|_|_|/ / | | | |/| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | I've changed the Python version in our Pipfile to Python 3.8. The main advantage of Python 3.8 is that it comes with significant upgrades to the unittest module which allow us to test asyncio-based code more easily. While our current test suite runs in P3.8 "out of the box", it currently still relies on many of the workarounds we had to use to test asynchronous code in Python 3.7. A future commit will replace these workarounds with the new tools available in Python 3.8. This commit also updates our discord.py version to 1.3.2. Versions of discord.py <= 1.3.1 contain a bug that causes errors in the new unittest tools that come with Python 3.8. For more specific details, see https://github.com/Rapptz/discord.py/pull/2570.
| | | | | | | | * Fix typo in commentGravatar Mark2020-03-03-1/+1
| | | | | | | | | | | | | | | | | | | | | | | | | | | Co-Authored-By: Leon Sandøy <[email protected]>
| | | | | | | | * Merge remote-tracking branch 'origin/master' into ↵Gravatar MarkKoz2020-02-25-1156/+2657
| | | | | | | | |\ | |_|_|_|_|_|_|_|/ |/| | | | | | | | | | | | | | | | | bug/backend/b748/resolver-in-coro
* | | | | | | | | Merge pull request #781 from ↵Gravatar S. Co12020-02-25-6/+11
|\ \ \ \ \ \ \ \ \ | |/ / / / / / / / |/| | | | | | | | | | | | | | | | | | | | | | | | | | python-discord/bug/utils/bot-1c/reminder-unschedule-failure Reminders: don't cancel task if reminder is invalid when rescheduling
| * | | | | | | | Merge branch 'master' into bug/utils/bot-1c/reminder-unschedule-failureGravatar Joseph2020-02-25-551/+1704
| |\ \ \ \ \ \ \ \ | |/ / / / / / / / |/| | | | | | | |
* | | | | | | | | Merge pull request #711 from python-discord/bug/backend/b704/ready-missing-cacheGravatar Mark2020-02-23-536/+1657
|\ \ \ \ \ \ \ \ \ | |_|_|/ / / / / / |/| | | | | | | | Prevent the role syncer from wiping the database table during API latency
| * | | | | | | | Merge remote-tracking branch 'origin/master' into ↵Gravatar MarkKoz2020-02-23-621/+997
| |\ \ \ \ \ \ \ \ | |/ / / / / / / / |/| | | | | | | | | | | | | | | | | bug/backend/b704/ready-missing-cache
* | | | | | | | | Merge pull request #749 from python-discord/reminder-enhancementsGravatar Leon Sandøy2020-02-23-15/+47
|\ \ \ \ \ \ \ \ \ | |_|_|/ / / / / / |/| | | | | | | | Reminder Embed Enhancements & Whitelist Expansion
| * | | | | | | | Merge branch 'master' into reminder-enhancementsGravatar S. Co12020-02-23-607/+951
| |\ \ \ \ \ \ \ \ | |/ / / / / / / / |/| | | | | | | |
| * | | | | | | | Add missed signature reformat from reviewGravatar S. Co12020-02-15-1/+4
| | | | | | | | | | | | | | | | | | | | | | | | | | | Co-Authored-By: Mark <[email protected]>
| * | | | | | | | Add "cancel" as a reminder delete aliasGravatar S. Co12020-02-15-1/+1
| | | | | | | | |
| * | | | | | | | Add full capture of reason string to superstarify invocationGravatar S. Co12020-02-15-1/+2
| | | | | | | | |
| * | | | | | | | Styling & refactors from reviewGravatar S. Co12020-02-15-17/+29
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | * Refactor confirmation embed footer string generation to be more concise * Multiline long method calls * Refactor humanized delta f string generation for readability * Switch from `datetime.isoformat` to `dateutils.parser.isoparse` to align with changes elsewhere in the codebase (should be more robust) * Shift reminder channel whitelist to constants Co-Authored-By: Mark <[email protected]>
| * | | | | | | | Expand reminder channel whitelist to dev-contrib for non-staffGravatar S. Co12020-02-12-1/+3
| | | | | | | | | | | | | | | | | | | | | | | | | | | Add channel ID to config files
| * | | | | | | | Add reminder target datetime to footer of confirmation messageGravatar S. Co12020-02-12-5/+16
| | | | | | | | |
| * | | | | | | | Add reminder ID to footer of confirmation messageGravatar S. Co12020-02-12-5/+8
| | | | | | | | |
| | * | | | | | | Sync: code style refactoringGravatar MarkKoz2020-02-23-7/+10
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | * Convert diff namedtuple to dict outside the dict comprehension * Define long condition as a boolean instead of in the if statement * Pass role and user dicts to aiohttp normally instead of unpacking
| | * | | | | | | Bot: send empty cache warning to a webhookGravatar MarkKoz2020-02-21-3/+12
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | This is more visible than it would be if it was only logged. * Add a webhook for the dev-log channel to constants
| | * | | | | | | API: add comment explaining class attributesGravatar MarkKoz2020-02-19-0/+2
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Explain changes caused by 22a55534ef13990815a6f69d361e2a12693075d5.
| | * | | | | | | Sync: add confirmation timeout and max diff to configGravatar MarkKoz2020-02-19-16/+24
| | | | | | | | |
| | * | | | | | | Tests: fix unawaited error for MockAPIClientGravatar MarkKoz2020-02-12-6/+5
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | This error is due to the use of an actual instance of APIClient as the spec for the mock. recreate() is called in __init__ which in turn creates a task for the _create_session coroutine. The approach to the solution is to use the type for the spec rather than and instance, thus avoiding any call of __init__. However, without an instance, instance attributes will not be included in the spec. Therefore, they are defined as class attributes on the actual APIClient class definition and given default values. Alternatively, a subclass of APIClient could have been made in the tests.helpers module to define those class attributes. However, it seems easier to maintain if the attributes are in the original class definition.
| | * | | | | | | Sync tests: use async_test decoratorGravatar MarkKoz2020-02-12-94/+135
| | | | | | | | |
| | * | | | | | | Sync tests: test on_member_joinGravatar MarkKoz2020-02-12-0/+52
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Should PUT user's data or POST it if the user doesn't exist. ResponseCodeError should be re-raised if status code isn't a 404. A helper method was added to reduce code redundancy between the 2 tests.
| | * | | | | | | Sync tests: add a third role with a lower ID to on_member_update testGravatar MarkKoz2020-02-12-1/+2
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | This better ensures that roles are being sorted when patching.
| | * | | | | | | Sync tests: fix on_member_remove listener testGravatar MarkKoz2020-02-12-13/+6
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | The listener was changed earlier to simply set in_guild to False. This commit accounts for that in the test.
| | * | | | | | | Sync tests: assert that listeners are actually added as listenersGravatar MarkKoz2020-02-12-0/+14
| | | | | | | | |
| | * | | | | | | Sync tests: test sync commands require the admin permissionGravatar MarkKoz2020-02-12-1/+18
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | The sync commands should only run if the author has the administrator permission. * Add missing spaces after class docstrings * Add missing docstring to SyncCogCommandTests
| | * | | | | | | Tests: create TestCase subclass with a permissions check assertionGravatar MarkKoz2020-02-12-0/+34
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | The subclass will contain assertions that are useful for testing Discord commands. The currently included assertion tests that a command will raise a MissingPermissions exception if the author lacks permissions.
| | * | | | | | | Sync: only update in_guild field when a member leavesGravatar MarkKoz2020-02-12-16/+6
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | The member and user update listeners should already be detecting and updating other fields so by the time a user leaves, the rest of the fields should be up-to-date. * Dedent condition which was indented too far
| | * | | | | | | Sync tests: test sync users commandGravatar MarkKoz2020-02-12-0/+7
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | sync() should be called on the UserSyncer.
| | * | | | | | | Sync tests: test sync roles commandGravatar MarkKoz2020-02-12-0/+9
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | sync() should be called on the RoleSyncer.
| | * | | | | | | Sync tests: test Sync cog's on_user_updateGravatar MarkKoz2020-02-12-1/+42
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | A user should be patched only if the name, discriminator, or avatar changes.
| | * | | | | | | Sync tests: fix ID in endpoint for test_sync_cog_on_member_removeGravatar MarkKoz2020-02-12-1/+1
| | | | | | | | |
| | * | | | | | | Sync tests: test Sync cog's on_member_update for other attributesGravatar MarkKoz2020-02-12-0/+21
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Members should not be patched if other attributes have changed.
| | * | | | | | | Sync tests: test Sync cog's on_member_update for rolesGravatar MarkKoz2020-02-12-0/+11
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Members should be patched if their roles have changed.
| | * | | | | | | Sync tests: test Sync cog's on_member_removeGravatar MarkKoz2020-02-12-0/+17
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | A PUT request should be sent to set in_guild as False and update other fields.