Movatterモバイル変換


[0]ホーム

URL:


Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

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

Merged
Bibo-Joshi merged 83 commits intomasterfromrefactor-tests
Feb 11, 2023
Merged

Conversation

@harshil21
Copy link
Member

@harshil21harshil21 commentedDec 15, 2022
edited
Loading

Closes#3407. I did not include thereq marker, but if you'd like that can be easily added as well.

Also some new "features":

  • Dict(Ext)Bot now overridesget_me to return a cachedUser object, which is obtained from thebot_info fixture. This saves lots of requests to the realget_me every time we initialize a bot, app, or updater.
  • A test marked as flaky won't re-run if the test failed with a flood, or was marked to xfail anyway.
  • Some tests which make a lot of requests are now run concurrently instead.
  • The testing without dependencies part in the CI is now done in one go instead of starting separate runs

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.

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 :)
@harshil21harshil21 added enhancement ⚙️ testsaffected functionality: tests 🛠 refactorchange type: refactor labelsDec 15, 2022
@harshil21harshil21 added this to thev20.0 milestoneDec 15, 2022
@harshil21harshil21 marked this pull request as draftDecember 15, 2022 23:30
@harshil21
Copy link
MemberAuthor

harshil21 commentedDec 15, 2022
edited
Loading

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
Copy link
Member

@Bibo-JoshiBibo-Joshi left a 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 …

@harshil21
Copy link
MemberAuthor

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 …

I think it looks like that since I had to rearrange those tests into these classes. Nevertheless i'll check again

@harshil21harshil21 added the 📋 pending-replywork status: pending-reply labelFeb 1, 2023
@Bibo-JoshiBibo-Joshi removed this from thev20.1 milestoneFeb 3, 2023
@harshil21harshil21 removed the 📋 pending-replywork status: pending-reply labelFeb 5, 2023
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
@harshil21harshil21 added the 📋 pending-mergework status: pending-merge labelFeb 10, 2023
@Bibo-JoshiBibo-Joshi merged commit963edbf intomasterFeb 11, 2023
@Bibo-JoshiBibo-Joshi deleted the refactor-tests branchFebruary 11, 2023 09:45
@Bibo-Joshi
Copy link
Member

Awesome work! 💪

@Bibo-JoshiBibo-Joshi removed the 📋 pending-mergework status: pending-merge labelFeb 11, 2023
@github-actionsgithub-actionsbot locked and limited conversation to collaboratorsFeb 19, 2023
@harshil21harshil21 added this to thev20.2 milestoneMar 25, 2023
@Bibo-JoshiBibo-Joshi added 🔌 enhancementpr description: enhancement and removed enhancement labelsNov 3, 2024
Sign up for freeto subscribe to this conversation on GitHub. Already have an account?Sign in.

Reviewers

@Bibo-JoshiBibo-JoshiBibo-Joshi requested changes

Assignees

No one assigned

Labels

🔌 enhancementpr description: enhancement🛠 refactorchange type: refactor⚙️ testsaffected functionality: tests

Projects

None yet

Milestone

v20.2

Development

Successfully merging this pull request may close these issues.

[REFACTOR] Split test classes into two:TestXXXNoRequest &TestXXXRequest

3 participants

@harshil21@Bibo-Joshi

[8]ページ先頭

©2009-2025 Movatter.jp