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

fix tests #15159

Merged
merged 13 commits into from
Apr 25, 2024
Merged

fix tests #15159

merged 13 commits into from
Apr 25, 2024

Conversation

pinzart90
Copy link
Contributor

@pinzart90 pinzart90 commented Apr 18, 2024

Refactor tests to use the test infrastructure properly (Setup and CLeanup)
Force Dispatcher to run all tasks before test ends (for most cases)
Refactor some SyncContext defaults to use the current one

Copy link

github-actions bot commented Apr 18, 2024

UI Smoke Tests

Test: success. 2 passed, 0 failed.
TestComplete Test Result
Workflow Run: UI Smoke Tests
Check: UI Smoke Tests - net8.0

@@ -84,6 +89,11 @@ public virtual void Start()
View.Show();

SynchronizationContext.SetSynchronizationContext(new DispatcherSynchronizationContext());
Copy link
Contributor

Choose a reason for hiding this comment

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

What will the current thread be typically in which this test fixture is set up?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We run all our tests in a single thread (UI thread).
We set the current syncContext the same way WPF would set when running the application normally (DispatcherSynchronizationContext)

Copy link
Contributor

Choose a reason for hiding this comment

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

It's surprising to me that if the tests are running in the UI thread, we have to do this explicitly. Perhaps this is a subtle difference between a main WPF app and a WPF test.

Comment on lines +167 to +171
var automation = this.ViewModel.Automation;
if (automation != null)
{
automation.PlaybackStateChanged -= OnAutomationPlaybackStateChanged;
}
Copy link
Contributor

Choose a reason for hiding this comment

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

It would be clearer if you refactored this into a corresponding UnregisterCommandCallback method to match with the RegisterCommandCallback method.

Copy link
Contributor

@aparajit-pratap aparajit-pratap left a comment

Choose a reason for hiding this comment

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

Some comments, then LGTM!

@pinzart90
Copy link
Contributor Author

Harmony fails on all builds.
Merging

@pinzart90 pinzart90 merged commit 03b6181 into master Apr 25, 2024
22 of 23 checks passed
@pinzart90 pinzart90 deleted the test_fixes branch April 25, 2024 19:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants