| Commit message (Collapse) | Author | Age | Lines |
|---|
| ... | |
| |\ \ \ \ \ \ \ \ \ \ \
| | | | | | | | | | | |
| | | | | | | | | | | | |
Migrate bot to Python 3.8
|
| | |\ \ \ \ \ \ \ \ \ \ \
| |/ / / / / / / / / / /
|/| | | | | | | | | | | |
|
| |\ \ \ \ \ \ \ \ \ \ \ \
| | | | | | | | | | | | |
| | | | | | | | | | | | | |
Add grabify (IP logger) domains to banned domains
|
| |/ / / / / / / / / / / / |
|
| |\ \ \ \ \ \ \ \ \ \ \ \
| | | | | | | | | | | | |
| | | | | | | | | | | | | |
Optimize Dockerfile for faster rebuilds
|
| | |\ \ \ \ \ \ \ \ \ \ \ \
| |/ / / / / / / / / / / /
|/| | | | | | | | | | | | |
|
| | | | | | | | | | | | | | |
|
| | | | | | | | | | | | | |
| | | | | | | | | | | | |
| | | | | | | | | | | | | |
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.
|
| | | |\ \ \ \ \ \ \ \ \ \ \
| |_|/ / / / / / / / / / /
|/| | | | | | | | | | | |
| | | | | | | | | | | | | |
I have resolved a merge conflict in Pipfile.lock by relocking with the newly added dependencies of `coloredlogs` and `colorama`.
|
| |\| | | | | | | | | | | |
| |_|_|_|/ / / / / / / /
|/| | | | | | | | | | | |
Coloured log output
|
| | | | | | | | | | | | | |
|
| |/ / / / / / / / / / /
| | | | | | | | | | |
| | | | | | | | | | |
| | | | | | | | | | |
| | | | | | | | | | | |
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.
|
| | |\ \ \ \ \ \ \ \ \ \
| |/ / / / / / / / / /
|/| | | | | | | | | |
| | | | | | | | | | | |
I've resolved the merge conflict by confirming the deleted part of tests/helpers.py
|
| |\ \ \ \ \ \ \ \ \ \ \
| | | | | | | | | | | |
| | | | | | | | | | | |
| | | | | | | | | | | |
| | | | | | | | | | | | |
python-discord/bug/backend/b754/scheduler-suppresses-errors
Don't suppress all errors in scheduler
|
| | |\ \ \ \ \ \ \ \ \ \ \
| |/ / / / / / / / / / /
|/| | | | | | | | | | | |
|
| | | | | | | | | | | | | |
|
| | | | | | | | | | | | |
| | | | | | | | | | | |
| | | | | | | | | | | |
| | | | | | | | | | | |
| | | | | | | | | | | | |
Logging it ourselves has a cleaner traceback and gives more control
over the output, such as including the task ID.
|
| | | | | | | | | | | | |
| | | | | | | | | | | |
| | | | | | | | | | | |
| | | | | | | | | | | |
| | | | | | | | | | | | |
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.
|
| | |\ \ \ \ \ \ \ \ \ \ \
| | | | | | | | | | | | |
| | | | | | | | | | | | |
| | | | | | | | | | | | | |
bug/backend/b754/scheduler-suppresses-errors
|
| | | | | | | | | | | | | |
| | | | | | | | | | | | |
| | | | | | | | | | | | |
| | | | | | | | | | | | |
| | | | | | | | | | | | |
| | | | | | | | | | | | |
| | | | | | | | | | | | |
| | | | | | | | | | | | |
| | | | | | | | | | | | | |
Main concern is someone trying to cancel a task directly. The workaround
for the race condition relies on the task only being cancelled via
Scheduler.cancel_task(), particularly because it removes the task from
the dictionary. The done callback will not remove from the dictionary
if it sees the task has already been cancelled. So it's a bad idea to
cancel tasks directly...
|
| | | | | | | | | | | | | |
| | | | | | | | | | | | |
| | | | | | | | | | | | |
| | | | | | | | | | | | |
| | | | | | | | | | | | | |
Because deactivate_infraction() explicitly cancels the scheduled task,
it now runs in a separate task to avoid prematurely cancelling itself.
|
| | | | | | | | | | | | | |
| | | | | | | | | | | | |
| | | | | | | | | | | | |
| | | | | | | | | | | | |
| | | | | | | | | | | | |
| | | | | | | | | | | | |
| | | | | | | | | | | | |
| | | | | | | | | | | | | |
This design makes more sense and is more convenient than requiring tasks
to be responsible for cancelling themselves.
* Rename _handle_task_exception to _task_done_callback
* Add trace logging
|
| | | | | | | | | | | | | |
| | | | | | | | | | | | |
| | | | | | | | | | | | |
| | | | | | | | | | | | |
| | | | | | | | | | | | |
| | | | | | | | | | | | |
| | | | | | | | | | | | | |
Explicitly retrieves the task's exception, which will raise the
exception if one exists.
* Rename _suppress_cancelled_error to _handle_task_exception
|
| | | | | | | | | | | | | |
| | | | | | | | | | | | |
| | | | | | | | | | | | |
| | | | | | | | | | | | |
| | | | | | | | | | | | |
| | | | | | | | | | | | |
| | | | | | | | | | | | | |
asyncio.create_task() exists and will already use the running loop in
the current thread. Because there is no intention of using a different
loop in a different thread anywhere in the program for the foreseeable
future, the loop parameter is redundant.
|
| | | | | | | | | | | | | | |
|
| | | | | | | | | | | | | |
| | | | | | | | | | | | |
| | | | | | | | | | | | |
| | | | | | | | | | | | |
| | | | | | | | | | | | |
| | | | | | | | | | | | |
| | | | | | | | | | | | |
| | | | | | | | | | | | | |
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()
|
| | | | | | | | | | | | | | |
|
| | | | | | | | | | | | | |
| | | | | | | | | | | | |
| | | | | | | | | | | | |
| | | | | | | | | | | | |
| | | | | | | | | | | | |
| | | | | | | | | | | | |
| | | | | | | | | | | | |
| | | | | | | | | | | | |
| | | | | | | | | | | | |
| | | | | | | | | | | | |
| | | | | | | | | | | | |
| | | | | | | | | | | | |
| | | | | | | | | | | | |
| | | | | | | | | | | | |
| | | | | | | | | | | | |
| | | | | | | | | | | | |
| | | | | | | | | | | | |
| | | | | | | | | | | | |
| | | | | | | | | | | | |
| | | | | | | | | | | | |
| | | | | | | | | | | | |
| | | | | | | | | | | | |
| | | | | | | | | | | | |
| | | | | | | | | | | | |
| | | | | | | | | | | | |
| | | | | | | | | | | | |
| | | | | | | | | | | | |
| | | | | | | | | | | | |
| | | | | | | | | | | | |
| | | | | | | | | | | | |
| | | | | | | | | | | | |
| | | | | | | | | | | | |
| | | | | | | | | | | | |
| | | | | | | | | | | | |
| | | | | | | | | | | | |
| | | | | | | | | | | | |
| | | | | | | | | | | | |
| | | | | | | | | | | | |
| | | | | | | | | | | | | |
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.
|
| | | | | | | | | | | | | |
| | | | | | | | | | | | |
| | | | | | | | | | | | |
| | | | | | | | | | | | |
| | | | | | | | | | | | | |
Autospec supports using AsyncMocks in 3.8 so there's no need to rely on
a subclass of CustomMockMixin for the async mocks.
|
| | | | | | | | | | | | | |
| | | | | | | | | | | | |
| | | | | | | | | | | | |
| | | | | | | | | | | | | |
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.
|
| | | | | | | | | | | | | |
| | | | | | | | | | | | |
| | | | | | | | | | | | |
| | | | | | | | | | | | | |
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".
|
| | | | |\ \ \ \ \ \ \ \ \ \ |
|
| | | | | | | | | | | | | | | |
|
| | | | | | | | | | | | | | |
| | | | | | | | | | | | | |
| | | | | | | | | | | | | |
| | | | | | | | | | | | | | |
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.
|
| | | | | | | | | | | | | | |
| | | | | | | | | | | | | |
| | | | | | | | | | | | | |
| | | | | | | | | | | | | |
| | | | | | | | | | | | | |
| | | | | | | | | | | | | | |
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.
|
| | | | | | | | | | | | | | |
| | | | | | | | | | | | | |
| | | | | | | | | | | | | |
| | | | | | | | | | | | | |
| | | | | | | | | | | | | |
| | | | | | | | | | | | | | |
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.
|
| | | | | | | | | | | | | | |
| | | | | | | | | | | | | |
| | | | | | | | | | | | | |
| | | | | | | | | | | | | | |
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.
|
| | | | | | | | | | | | | | |
| | | | | | | | | | | | | |
| | | | | | | | | | | | | |
| | | | | | | | | | | | | | |
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.
|
| | | | | | | | | | | | | | |
| | | | | | | | | | | | | |
| | | | | | | | | | | | | |
| | | | | | | | | | | | | |
| | | | | | | | | | | | | |
| | | | | | | | | | | | | | |
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.
|
| | | | | | | | | | | | | | |
| | | | | | | | | | | | | |
| | | | | | | | | | | | | |
| | | | | | | | | | | | | |
| | | | | | | | | | | | | |
| | | | | | | | | | | | | | |
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.
|
| | | | | | | | | | | | | | |
| | | | | | | | | | | | | |
| | | | | | | | | | | | | | |
Co-Authored-By: Leon Sandøy <[email protected]>
|
| | | | | | |\ \ \ \ \ \ \ \ \
| | | |_|_|/ / / / / / / / /
| | |/| | | | | | | | | | |
| | | | | | | | | | | | | | |
bug/backend/b748/resolver-in-coro
|
| | | | | | | | | | | | | | | |
|
S. Co1
Leon Sandøy
kwzrd
Joseph
scragly
Spencer Young
S. Co1
Sebastiaan Zeeff
Eivind Teig
MarkKoz