Skip to content
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 our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Run individual pytests instead of running all pytests at once. #2350

Merged
merged 1 commit into from
Jul 12, 2022

Conversation

mint570
Copy link
Contributor

@mint570 mint570 commented Jun 23, 2022

What I did
Run pytests one test at a time.

Why I did it
Fix #2330

How I verified it

Details if related

Change-Id: Ibe02f3fe47846c89c50da1d8e97a78739c2275f8
@mint570
Copy link
Contributor Author

mint570 commented Jun 24, 2022

@mint570
Copy link
Contributor Author

mint570 commented Jun 24, 2022

@lguohan

@mint570
Copy link
Contributor Author

mint570 commented Jun 27, 2022

@prsunny @pettershao-ragilenetworks
Can you please take a look at this PR?

@mint570 mint570 changed the title Run individule pytests instead of running all pytests at once. Run individual pytests instead of running all pytests at once. Jul 6, 2022
@mint570
Copy link
Contributor Author

mint570 commented Jul 6, 2022

@theasianpianist can you please take a look?

@prsunny
Copy link
Collaborator

prsunny commented Jul 9, 2022

@theasianpianist , please review

@prsunny
Copy link
Collaborator

prsunny commented Jul 9, 2022

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@prsunny
Copy link
Collaborator

prsunny commented Jul 12, 2022

lgtm, @liushilongbuaa , @xumia , could you please review?

Copy link
Contributor

@liushilongbuaa liushilongbuaa left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@prsunny
Copy link
Collaborator

prsunny commented Jul 13, 2022

@mint570 , as discussed offline with @theasianpianist , we see a significant increase in time for the test completion.

image

Can you please work on an alternative solution for #2373 and #2330?

@mint570 mint570 deleted the fix_cov branch July 13, 2022 18:43
@mint570
Copy link
Contributor Author

mint570 commented Jul 13, 2022

I don't have an obvious solution now. The increase in time is mostly due to the overhead of running individual test.
There might be some improvement we can do. Such as grouping the tests together (instead of running 100 tests, we can run 10 test groups, each includes 10 tests). But the overall time will still be increased compare to running all tests at once.

@mint570
Copy link
Contributor Author

mint570 commented Jul 14, 2022

Some random ideas. May or may not help:

  1. Integrate the coverage file collection into the pytest. So that we can run all tests in one pytest call.
  2. Share a single vs container with all tests.

Not sure if the above is possible or not. I tried to look at how the pytest dvs works. But unfortunately, I don't have enough expertise to understand that.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Pytest runs out of memory when coverage is enabled
5 participants