- Notifications
You must be signed in to change notification settings - Fork5.9k
Refactor and overhaul the test suite#3426
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
Uh oh!
There was an error while loading.Please reload this page.
Conversation
This commit additionally changes:- The scope of the fixtures to module- Class variables are moved into a namespace class called as Space- Auto addition of flaky and no_req marker (in conftest.py)- Caching of default and tz bots (in conftest.py)Apart from this, no tests were added/deleted/modified in any way.
Also adapt the bot tests for this change + removes debug prints + fix few tests
Also change the scope of some fixtures
This mainly does two things:- Leverages the session bot while making new applications, this eliminates all calls to get_me- `test_signal_handler` was optimized to use `loop.call_later` which eliminates the chance where we could get a TaskException not retrieved error, which often fatally crash pytest.These two changes thus reduce flakyness and improves speed.
The last commit addresses the root cause of its addition, so now we don't need this :)
harshil21 commentedDec 15, 2022 • 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.
These tests don't fail for me locally, will have a look later. I suspect it is due to the change of scope of those fixtures. Edit: See commit message below for details on why it failed. |
This was done as when running pytest with -x, this error is emitted. Not doing this seems to fix the problem. However this could backfire on python versions older than 3.10.6
this is based on whether the test calls get_me indirectly or not
…tinefunctionThe limitation of that function is fixed by using the `__wrapped__` attribute
…d/create_tasklet's take full advantage of the lib now being async
includes removing unused params, removing unnecessary monkeypatch.delattr's, missing import, beautification, etc
…llback testsThis means we can now move those methods back to TestBotNoReq, since they did not make any request apart from delete_webhook()
The tests are roughly arranged as follows:- test_slot_behaviour- test de_json/to_dict- test init- test equality- test wrong init- etc
Also add a missing test_to_dict in user.py
Bibo-Joshi left a comment
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.
I reviewd the commits since my last review. In a few places it looks like you duplicated an already exsting tests. Maybe those are merge errors or maybe it only looks that way if you don't viewall commits …
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.
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.
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.
Uh oh!
There was an error while loading.Please reload this page.
harshil21 commentedJan 22, 2023
I think it looks like that since I had to rearrange those tests into these classes. Nevertheless i'll check again |
specially on python 3.7. The xdist_group markers are kept so we can use them locally
Also increase loop.call_later call by 0.1 seconds in test_signal_handlers
Bibo-Joshi commentedFeb 11, 2023
Awesome work! 💪 |
Uh oh!
There was an error while loading.Please reload this page.
Closes#3407. I did not include the
reqmarker, but if you'd like that can be easily added as well.Also some new "features":
Dict(Ext)Botnow overridesget_meto return a cachedUserobject, which is obtained from thebot_infofixture. This saves lots of requests to the realget_meevery time we initialize a bot, app, or updater.Many tests were optimized as well, which should decrease the flakyness in the CI.
How do I review this?!
It is best to review by commit, the changes are specific and I rebased multiple times to make sure changes fit in a certain category.