| Commit message (Collapse) | Author | Lines |
|
Co-Authored-By: Leon Sandøy <[email protected]>
|
|
Resolves an issue mentioned in https://github.com/python-discord/bot/issues/767, giving Helpers access to post invites and other things caught by the Filtering cog.
|
|
|
|
Previous regex utilized a `/`, which doesn't work for comparing against Windows paths, which use `\`
|
|
Returning directly out of a `finally` clause can cause any exceptions raised in the clause to be discarded, so we can remove the finally clause entirely and shift the control statements into the body of the function
|
|
* Remove explicit urllib3 pinning, CVE that caused its pinning has been resolved by 1.25+. This is a child dependency of requests.
|
|
|
|
|
|
This makes it easy to add colour to the logs. Colorama is also installed
if on a Windows system.
|
|
I've migrated the `tests/test_snekbox.py` file to use the new Python 3.8-style unittests instead of our old style using our custom Async mocks.
In particular, I had to make a few changes:
- Mocking the async post() context manager correctly
Since `ClientSession.post` returns an async context manager when called, we need to make sure to assign the return value to the __aenter__ method of whatever `post()` returns, not of `post` itself (i.e.. when it's not called).
- Use the new AsyncMock assert methods `assert_awaited_once` and `assert_awaited_once_with`
Objects of the new `unittest.mock.AsyncMock` class have special methods to assert what they were called with that also assert that specific coroutine object was awaited. This means we test two things in one: Whether or not it was called with the right arguments and whether or not the returned coroutine object was then awaited.
- Patch `functools.partial` as `partial` objects are compared by identity
When you create two partial functions of the same function, you'll end up with two different `partial` objects. Since `partial` objects are compared by identity, you can't compare a `partial` created in a test method to that created in the callable you're trying to test. They will always compare as `False`. Since we're not interested in actually creating `partial` objects, I've just patched `functools.partial` in the namespace of the module we're testing to make sure we can compare them.
|
|
|
|
|
|
It could have caused some errors if the user delete his own message
|
|
|
|
Logging it ourselves has a cleaner traceback and gives more control
over the output, such as including the task ID.
|
|
|
|
A task should not be cancelled if an infraction is permanent because
tasks don't exist for permanent infractions.
Fixes BOT-1V
|
|
Unicode literals aren't really safe compared to code points
|
|
Two functions were created: send_eval and continue_eval, in order to facilitate testing. The corresponding tests are also changed in this commit.
|
|
|
|
|
|
|
|
|
|
|
|
This case is already covered by checking if at least one letter is included.
|
|
The tag fallback didn't convert tags, resulting in possible invalid tag names being passed to the `tags_get_command`. This makes sure they're valid and ignores the risen exception if they are not.
|
|
With only ascii and numbers being allowed to go through, possible values still included things like `$()` which don't match anything in `REGEX_NON_ALPHABET` from tags.py resulting in an error.
|
|
|
|
With `<` the check only went through when the token was already expired, making revoking redundant; and didn't go through when the token still had some time before expiration.
|
|
|
|
After the change to also check empty strings to avoid unucessary requests, it is no longer necessary to do an explicit value check, as the only values that can come from the .env file are `None` and strings
Co-authored-by: Karlis S <[email protected]>
|
|
|
|
executed.
|
|
If a task is cancelled it is assumed it was done via cancel_task. That
method deletes the task after cancelling so the warning isn't relevant.
|
|
Task.exception() only returns the exception. It still needs to be
explicitly raised.
|
|
To prevent a deletion of task rescheduled with the same ID, the callback
checks that the stored task is the same as the done task being handled.
* Only delete the task; it doesn't need to be cancelled because the it
is already done
* Revise the callback's docstring to explain the new behaviour
* Rename `task` parameter to `done_task`
|
|
* Use imperative mood for docstring
* Explain the purpose of the parameter in the docstring
* Make log message after cog name lowercase
|
|
The shield exists to be used for exactly this purpose so its a better
fit than create_task.
|
|
Without the check and an invalid token, an AttributeError is raised; blocking the relevant ClientError from being raised in `get_access_token`.
|
|
Docker fetches values from the .env itself and defaults to "" instead of None, needing to do invalid access token requests before unloading itself.
|
|
|
|
The previous implementation assumed the config class was a subsection, failing with a KeyError if it wasn't one.
Co-authored-by: kwzrd <[email protected]>
|
|
|
|
Delete duplicate keys that were missed in the merge
|
|
Co-authored-by: SebastiaanZ <[email protected]>
|
|
Co-authored-by: SebastiaanZ <[email protected]>
|
|
When environment variables weren't provided; the cog attempted to create a BasicAuth object with None as values resulting in an exception before the event loop was started and a subsequent crash.
|
|
The function was only used in the since removed `Events` cog.
|
|
|
|
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.
|