-
Notifications
You must be signed in to change notification settings - Fork 255
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
Fix bugs in parent class init/cleanup logic #660
Fix bugs in parent class init/cleanup logic #660
Conversation
… class cleanup on top level class
…level class cleanup
…nit/cleanup methods
@parrainc FYI since this fixes some bugs from your change in case you have feedback. |
test/UnitTests/MSTest.CoreAdapter.Unit.Tests/Execution/TestClassInfoTests.cs
Outdated
Show resolved
Hide resolved
We're seeing some problems with this change (we needed the same fix, so we've got a build of it working). The tests work fine locally, but our CI build (running in Azure Pipelines) runs for anything between 30 and 90 minutes, and then either the node is considered unresponsive so is terminated, or we error with
We've updated the Test Tools in use to 16.4 (latest stable) but that doesn't seem to make a difference. Sorry I can't be more help with this. I appreciate you're all on holiday at the moment, so I'm probably going to see what I can find out tomorrow and I'll update this Issue if I find anything |
Turns out that the scenario that these changes had enabled for us exposed a flaw in our tests that had runaway memory usage, so the problem was in our code! I think it just exhausted the memory on the Pipeline servers until paging stuff in and out to disk caused them to be unresponsive. When we got it working on a locally installed build agent it topped out at 24gb of RAM in use! |
I think already provided review comments cover pretty much it. For me, it looks good! Thanks for addressing that issue :) |
Fixing two bugs related to feature #143