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

Fixing problems with tests running with XUnit StaFact #259

Merged
merged 2 commits into from
Apr 17, 2021

Conversation

eriove
Copy link
Contributor

@eriove eriove commented Apr 1, 2021

I ran into a problem where my tests were failing when updating to version 4.30.0 or later. I got an exception in DockingManager.OnContextMenuPropertyChanged(). It was introduced as part of
#170 to solve a problem with the context menu.

Here is a link to the specific change (you will have to expand the diff of DockingManager to see the row).

My problem is that I'm running tests with the DockingManager in XUnit with StaFact. StaFact makes sure that the tests run on on STA thread. But it doesn't guarantee that all tests within a class runs on the same STA thread. Hence d will have the test thread's dispatcher, but for some reason the contextMenu will have a dispatcher from a thread from a previous test (from within the same test class).

This is a hack (and I'm not sure if it should be merged) but the only way I can think of resolving this right now. @LyonJack (or someone else) do you have any ideas why the context menu end up on another thread? Is it somehow reused for multiple docking managers? Dependency properties are static so I can see how that could happen.

@eriove
Copy link
Contributor Author

eriove commented Apr 12, 2021

I've finally had time to dig deeper into this problem and updated the pull request.

The problem is that the styles are statically cached and will be re-used between tests. If different tests run on different threads they will crash as described above. I've created a minimal test that reproduces the problem, it is available here.

By changing the context menus from static to dynamic resources the tests pass, but there is some extra resource allocation. @Dirkster99, do you think that that would an acceptable trade off?

@Dirkster99 Dirkster99 merged commit f59abc4 into Dirkster99:master Apr 17, 2021
@eriove eriove deleted the feature/handle-threads-in-tests branch April 17, 2021 20:17
@eriove
Copy link
Contributor Author

eriove commented Apr 17, 2021

Thanks for merging!

@Dirkster99
Copy link
Owner

Thanx for contributing :-)

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.

2 participants