Implement WakeLock so that the system does not enter sleep #2670
Conversation
timcassell
left a comment
timcassell
left a comment
There was a problem hiding this comment.
I appreciate all the tests! Do any other OSes besides Windows need this treatment?
leonvandermeer
force-pushed
the
feature/wake-lock
branch
from
e3eb631 to
242589b
Compare
leonvandermeer
force-pushed
the
feature/wake-lock
branch
from
242589b to
d774258
Compare
timcassell
left a comment
timcassell
left a comment
There was a problem hiding this comment.
LGTM! I would like a second review before merging this new functionality, though. @adamsitnik or @AndreyAkinshin may have different opinions on what the default should be.
…hmarks are running
Co-authored-by: Tim Cassell <[email protected]>
leonvandermeer
force-pushed
the
feature/wake-lock
branch
from
d774258 to
75b75c9
Compare
|
Hi @AndreyAkinshin and @adamsitnik . Happy New Year1! Do you have time to look at this feature? I will be very happy if my contribution makes it into the product. Footnotes
|
adamsitnik
left a comment
adamsitnik
left a comment
There was a problem hiding this comment.
@leonvandermeer big thanks for your high quality contribution!
Overall the code looks good, but some feedback needs to be addressed before the code gets merged. PTAL at my comments.
Co-authored-by: Adam Sitnik <[email protected]>
|
FYI, I'm finished with this rework round. Please take next steps to merge the feature. |
|
It looks to me like all the feedback was addressed, so I will go ahead and merge. I actually recently ran into this issue on a MacBook, so hopefully someone can extend this to other OSes in the future. Thanks @leonvandermeer! |
adamsitnik
left a comment
adamsitnik
left a comment
There was a problem hiding this comment.
Thank you for addressing my feedback and providing high quality contribution to BenchmarkDotNet @leonvandermeer 👍
| { | ||
| logger.WriteLineError($"Unable to allow the system from entering sleep or turning off the display (error message: {ex.Message})."); | ||
| } | ||
| safePowerHandle.Dispose(); |
There was a problem hiding this comment.
the disposal should be put into a finally block
3 participants
Implement WakeLock so that the system does not enter sleep while benchmarks are running.
As a happy user of BDN, I was annoyed by the fact that long running benchmarks were not completed when I returned to my system. All because my system enters sleep, after not being touched for a while.
The test code needs to enumerate power requests. I decided not to pinvoke undocumented api as maintenance is difficult if it changes in future windows version. It gave me a chance to learn more about parsers in general 😂. Imo, maintaining the parser when
powercfg /requestsoutput would change is less difficult.In case you prefer usage of Superpower library to parse, I'm willing to change that.
All test code is in integration tests, since
powercfgneeds elevation. I imagine developers dislike failing unit tests when they run those locally.