testfx
testfx copied to clipboard
microsoft
Proposal: UIDataTestMethod
The DataTestMethod attribute is really helpful and makes writing tests a lot easier. However when developing tests that need to run on the UI thread, we have to use the UITestMethod attribute. This however leaves us with having to implement data driven tests ourselfes. Having a UIDataTestMethod attribute would be great to have to fill the gap between UI tests and non UI tests.
Where would this actually be contributed to? Does UITestMethod even live in this repository?
@chingucoding it does live in this repo. https://github.com/microsoft/testfx/blob/ec18af6f90c272f68f48d9d0b94c71b8e276c7a0/src/TestFramework/Extension.UWP/UITestMethodAttribute.cs#L12-L43
Also on a similar note, I am working on the Windows Community Toolkit and we make heavy use of UITestMethod either creating this UIDataTestMethod would be very useful or maybe a better solution would be to make using UITestMethod and DataRow together work, rather than make a new attribute?
Currently using those attributes together causes this rather obscure error.
Oh that's interesting, thanks for pointing that out @RosarioPulella!
I'm also not sure what the best move forward would be. I think having a UIDataTestMethod would be best for API simplicity since it extends on an existing pattern. I also got the issue you linked often and was confused what was happening. This error message should definitely be updated.
I think having a UIDataTestMethod would be best for API simplicity since it extends on an existing pattern.
I am not in tune with a lot of the patterns of this API, but I see a lot of test methods decorated with DataRow and a simple TestMethod and it works fine. It also seems like there is an existing pattern to compose DataRow with other attributes than DataTestMethod.
Either way it goes, I would like something to open up for using DataRow in UI tests.
Looks like you have a better view of how the properties work together. If DataRow works with normal TestMethod, why exactly is there a DataTestMethod attribute though? I though you need to have those and otherwise it wouldn't work.
Totally agree that no matter what we settle on, the situation needs to change. Having no DataRow support in UI tests is quite annoying.
@chingucoding they DataTestMethod used to be a separate thing, but they merged them and just left the old one there for compatibility for folks. It should probably just be marked Obsolete so folks could move off of it and they could remove it in the future.
We hit the same problem with async as well, but using a TestMethod to just dispatch to the UI thread first things seems to be working. Same for doing a similar thing for DataRow as well. It's just a bit more verbose.
DataTestMethodAttribute has no functionality, we keep it there for compability. It's same as TestMethodAttribute. Instead of adding a new method, we should fix the bug in UITestMethod.
@Sergio0694 your dispatcherqueue extensions in the toolkit test if it needs to be dispatched first, eh? So it's probably a similar type of helper needed in DataRow, so if it's used with the UITestMethod test, then it'll just dispatch the call as needed to where the test is running? I think?
"your dispatcherqueue extensions in the toolkit test if it needs to be dispatched first, eh"
That is correct. Though note that that was just meant to be a small performance optimization for when the extension was needlessly called, and it should not result in any functional changes compared to someone just always using just TryEnqueue. That is to say, the fact that those extensions do that should not (I think) affect how or when they're being used. It's just that, whenever they're used, they also try to save some more memory and CPU time if they can, otherwise just forward to TryEnqueue as usual 😄
Was trying to test this inside the project by adding a UWP test app and have the use the libraries from the repository, however I can not build the project as it fails with the following error message:
1>CSC : error CS7032: Key file 'D:\Projects\testfx\scripts\build\key.snk' is missing the private key needed for signing
Maybe you guys know more about this or know a way to work around this?
@chingucoding I believe that path is stored in one of the project or manifest files, you should just be able to remove that line so it doesn't try and use the non-existent snk file?
@chingucoding I believe that path is stored in one of the project or manifest files, you should just be able to remove that line so it doesn't try and use the non-existent snk file?
If I do that, the issue get's even worse as other projects also fail to build...
@chingucoding did you get anywhere with this? We're hitting this again with Toolkit Labs for Windows, so it's a bit of a pain. It requires us to use TestMethod and then Dispatch to the UI Thread in each of our tests to start, instead of being able to just have that be handled automatically by the test harness. (As we're trying to await the content to test to be loaded.)
@pavelhorak saw this was added to a milestone is it something your team was also thinking of looking at? I tried to write our own TestMethod wrapper using DispatcherQueue for UWP as well, but ran into the same issue. The test app just hangs in a deadlock as soon as the first await is hit in test initialization or test code. (I'd also suggest renaming this issue to "support async methods with UITestMethodAttribute"; compared to the initial request and discussion, the end result is this missing feature.)
Could someone explain the underlying issue with the state machine of why this is currently not supported by the UITestMethodAttribute? @Sergio0694 I may ask your thoughts on this in the morning.
https://github.com/microsoft/testfx/blob/5862d22be4616058d5f78e0b46737d3af8f42956/src/TestFramework/Extension.UWP/UITestMethodAttribute.cs#L12-L30
@chingucoding I believe that path is stored in one of the project or manifest files, you should just be able to remove that line so it doesn't try and use the non-existent snk file?
If I do that, the issue get's even worse as other projects also fail to build...
This sounds like a key is needed for strong naming but the default .gitignore was used and this prevents .snk files from being checked in. (This is something I frequently see with projects on GitHub.) Creating a new key with that name/path will probably fix it. It won't fix it if the public key of the missing file is used somewhere but if that was the case I'd expect this to be a bigger issue for more people working on the codebase.
I tested, and I think the UITestMethod for WinUI 3 has the same issue. I was hoping since it's already using the new DispatcherQueue that it may be able to work.
What I don't understand is why the regular TestMethod attribute supports this? It's execute is just calling Invoke directly. Is it the extra GetAwaiter().GetResult() pattern that's locking the thread pool as the async/await pattern isn't being used by the test running itself?
@dotMorten, @azchohfi mentioned you may have found a workaround/solution to this by tweaking a custom attribute? Do you have that somewhere or was it in regard to something else?
@michael-hawker There's a few ways to do it. WinUIEx has a test tools package that uses a code-generator to pull it off with a simple tag. But you're right it can also be done with a custom TestMethodAttribute - I'll have to dig that out if you need it. I definitely don't use the mstest provided UITestMethod since it doesn't support async tests - my solution does. But the simplest way is to just use a dispatcher to switch to the UI thread like I'm doing in this WPF helper method here: https://github.com/dotMorten/UniversalWPF/blob/main/src/UnitTests/Framework/UIHelpers.WPF.cs#L55-L59 Then it's just a matter of awaiting that call like this: https://github.com/dotMorten/UniversalWPF/blob/main/src/UnitTests/RelativePanelTests.cs#L19-L20
