-
Notifications
You must be signed in to change notification settings - Fork 779
Fixes #4804. Unit Tests are not running in degraded mode #4805
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
Closed
Closed
Changes from 7 commits
Commits
Show all changes
24 commits
Select commit
Hold shift + click to select a range
2ce7f4e
Fixes #4804. InjectKey Tests with Pipeline Mode hangs forever in the …
BDisp be016ca
Code cleanup
BDisp 86f567a
Forced to use Console.KeyAvailable due a not reliable GetNumberOfCons…
BDisp f348350
Call ProcessQueue while _testSource IsAvailable is true
BDisp 9028e4e
Check AnsiPlatform in CI pipelines
BDisp 399370e
Add AnsiPlatform check to all pipeline mode
BDisp c92d71d
Add IsRunningInTest method
BDisp e79fcc7
Move static IsRunningInTest method to the internal DriverImpl class
BDisp 2a9c015
Merge branch 'v2_develop' into v2_4804_readfile-hang-fix
BDisp 30a4a3d
Increasing delay time
BDisp bbd4067
Merge branch 'v2_develop' into v2_4804_readfile-hang-fix
BDisp 5b2b46c
Put delay after call ProcessQueue
BDisp 762d304
Delay before and after call ProcessQueue
BDisp a906657
Remove faulty code not needed now because the options.AutoProcess is …
BDisp bd9da52
Increase the delay in the longer test
BDisp a35a38e
Delay after ProcessQueue
BDisp a388ac0
Add WaitUntilAsync helper method
BDisp ed091dc
Testing without delay
BDisp 7657a6a
Change AutoProcess = true
BDisp 4d97378
Rename to DisableDriverRealIO per @tig
BDisp ffcad71
Fix erroneous information
BDisp 0711d00
Fix erroneous information
BDisp 2e74d52
Revert changes
BDisp a1f2318
Skip tests
BDisp File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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'd much prefer this to be on IDriver. I worked hard to ensure there was no coupling from drivers to IApplication and this undoes that.
Can you refactor this such that it's a property on IDriver instead?
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.
Sure, I was just testing and I really thought it wasn't the proper way. So I'll include it in IDriver as a property and implement it in DriverImpl. Does that sound good to you?
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.
Thinking about it, this method isn't characteristic of IDriver because it refers to the type of application being executed. I think it's better to add it as a property in IApplication and implement it in ApplicationImpl. I think that would be the right way. What do you think?
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.
Sorry. Forget my previous comment. The reason it's included in IDriver is that practically all the classes that use it are also used by IDriver. So you were right in your reasoning. Thanks.
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.
The problem is that besides creating a property in IDriver and implementing it in DriverImpl, I'll also have to create the same property in IInput and IOutput. Don't you think that's too much work for just one property, which could even be static because when one test is running, all the others will run as well? I await your opinion and will work on the flaws in Linux and Mac.
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.
Because it's also used in several places. See the image below. If the IInput and IOutput interfaces had the IDriver property, then in their implementations it would be enough to call something like
if (Driver.IsRunningInTest). Then you also have the case of the static class ClipboardProcessRunner, which adds even more complexity. The UnixTerminalHelper class would be easier because it's only called by AnsiOutput and UnixOutput, and you would just need to add a new parameterbool isRunningInTestto its Suspend method. The most complicated to solve would be ClipboardProcessRunner because it doesn't have any reference to IDriver, IInput, and IOutput. With this explanation, see if you can find the best solution.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 thought this only impacts windows.
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.
That was my initial perception, but after the tests failed with all the drivers, I verified that all the low-level reading functions were actually being executed.
Unfortunately, I still don't understand why the tests are still failing. The API could actually be used without the need for Task.Delay, but rather by signaling an event. Now we just need to figure out where.
Uh oh!
There was an error while loading. Please reload this page.
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.
Try this: set an env variable in the yml. Have the IInput impls check for that variable.
There may already be an env variable set we can use.
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.
But the error is no longer due to low-level functions because they no longer execute with the configuration in the test projects. The problem now is that not all injected keys have yet been processed by the input processor.