From 58b65f8a8a39b7612b2caf014dc45fa6af5264fa Mon Sep 17 00:00:00 2001 From: Konstantina Chremmou Date: Sat, 12 Jan 2019 18:14:57 +0000 Subject: [PATCH] Enforce failure if MWWaitFor times out without a message, ortherwise failures are swallowed. Use optional parameters instead of method overloads. Some other modernisations. Signed-off-by: Konstantina Chremmou --- XenAdmin/Core/Win32Window.cs | 19 +- XenAdminTests/MainWindowTester.cs | 81 +++----- .../MainWindowWrapper/MainWindowWrapper.cs | 196 ++---------------- .../MainWindowWrapper/TestWrapper.cs | 48 ++--- XenAdminTests/MiscTests/WebBrowser2Tests.cs | 40 ++-- 5 files changed, 84 insertions(+), 300 deletions(-) diff --git a/XenAdmin/Core/Win32Window.cs b/XenAdmin/Core/Win32Window.cs index 16543aa95..defbda10b 100644 --- a/XenAdmin/Core/Win32Window.cs +++ b/XenAdmin/Core/Win32Window.cs @@ -121,33 +121,24 @@ namespace XenAdmin.Core } } - /// - /// Gets the first window that has the specified window text. - /// - /// The text that the window must have. - /// The first window that has the specified window text. - public static Win32Window GetWindowWithText(string text) - { - return GetWindowWithText(text, w => true); - } - /// /// Gets the first window that has the specified window text and matches the specified Predicate. /// /// The text that the window must have. /// The Predicate that the window must match. /// The first window that has the specified window text and matches the specified Predicate. - public static Win32Window GetWindowWithText(string text, Predicate match) + public static Win32Window GetWindowWithText(string text, Predicate match = null) { - Util.ThrowIfParameterNull(match, "match"); + if (match == null) + match = w => true; + Thread.Sleep(100); foreach (Win32Window window in TopLevelWindows) { if (window.Text == text && match(window)) - { return window; - } } + return null; } diff --git a/XenAdminTests/MainWindowTester.cs b/XenAdminTests/MainWindowTester.cs index a4cd8672d..dd29529ed 100644 --- a/XenAdminTests/MainWindowTester.cs +++ b/XenAdminTests/MainWindowTester.cs @@ -88,68 +88,59 @@ namespace XenAdminTests /// /// Waits for the specified action to return true. The action is run on the main program thread. - /// The action is attempted 300 times, with a 100ms wait between tries. If the action doesn't - /// succeed after being retried then the calling test is failed with the specified message. - /// If assert message is null, then the test won't be failed. + /// The action is attempted 300 times, with a 100ms wait between tries. + /// If the action has not succeeeded after the retires, the calling test fails with the specified message. /// /// The action. /// The assert message. /// A value indicating whether the action was successful. - protected bool MWWaitFor(Func action, string assertMessage) + protected bool MWWaitFor(Func action, string assertMessage = null) { bool success = false; - for (int i = 0; i < 500 && !success; i++) + for (int i = 0; i < 500; i++) { - success = MW(action); - if (!success) - { - Thread.Sleep(300); - } + success = MW(action); + if (success) + break; + + Thread.Sleep(300); } - if (assertMessage != null && !success) - { - Assert.Fail(assertMessage); - } + if (!success) + Assert.Fail(string.IsNullOrEmpty(assertMessage) ? "Waited unsuccessfully" : assertMessage); return success; } /// /// Waits for the specified action to return true. The action is run on the calling thread. - /// The action is attempted 300 times, with a 100ms wait between tries. If the action doesn't - /// succeed after being retried then the calling test is failed with the specified message. - /// If assert message is null, then the test won't be failed. + /// The action is attempted 300 times, with a 100ms wait between tries. + /// If the action has not succeeded after the retries, the calling test fails with the specified message. /// /// The action. /// The assert message. /// A value indicating whether the action was successful. - protected bool WaitFor(Func action, string assertMessage) + protected void WaitFor(Func action, string assertMessage = null) { bool success = false; - for (int i = 0; i < 300 && !success; i++) + for (int i = 0; i < 300; i++) { success = action(); - if (!success) - { - Thread.Sleep(100); - } + if (success) + break; + + Thread.Sleep(100); } - if (assertMessage != null && !success) - { - Assert.Fail(assertMessage); - } - return success; + if (!success) + Assert.Fail(string.IsNullOrEmpty(assertMessage) ? "Waited unsuccessfully" : assertMessage); } - internal Win32Window WaitForWindowToAppear(string windowText) + internal Win32Window WaitForWindowToAppear(string windowText, Predicate match = null) { - return WaitForWindowToAppear(windowText, w => true); - } + if (match == null) + match = w => true; - internal Win32Window WaitForWindowToAppear(string windowText, Predicate match) - { Win32Window window = null; Func func = delegate { @@ -157,30 +148,10 @@ namespace XenAdminTests return window != null; }; - WaitFor(func, "Window with text " + windowText + " didn't appear."); + WaitFor(func, $"Window with text {windowText} didn't appear."); return window; } - /// - /// Waits for the specified action to return true. The action is run on the main program thread. - /// The action is attempted 200 times, with a 100ms wait between tries. - /// - /// A value indicating whether the action was successful. - protected bool MWWaitFor(Func action) - { - return MWWaitFor(action, null); - } - - /// - /// Waits for the specified action to return true. The action is run on the calling thread. - /// The action is attempted 200 times, with a 100ms wait between tries. - /// - /// A value indicating whether the action was successful. - protected bool WaitFor(Func action) - { - return WaitFor(action, null); - } - /// /// doesn't seems to work from the test framework. This is a replacement. /// @@ -507,7 +478,7 @@ namespace XenAdminTests }); MW(openDialog); - MWWaitFor(() => closed, "Dialog \"" + windowText + "\" was not closed."); + MWWaitFor(() => closed, $"Dialog {windowText} was not closed."); } /// diff --git a/XenAdminTests/MainWindowWrapper/MainWindowWrapper.cs b/XenAdminTests/MainWindowWrapper/MainWindowWrapper.cs index b6baa19d9..72d4c78ef 100644 --- a/XenAdminTests/MainWindowWrapper/MainWindowWrapper.cs +++ b/XenAdminTests/MainWindowWrapper/MainWindowWrapper.cs @@ -29,16 +29,11 @@ * SUCH DAMAGE. */ -using System; -using System.Collections.Generic; -using System.Text; using XenAdmin; using System.Windows.Forms; using XenAdmin.Controls; -using XenAdmin.Controls.MainWindowControls; -using XenAdmin.Controls.XenSearch; using XenAdmin.Plugins; -using XenAdmin.Commands; + namespace XenAdminTests { @@ -49,193 +44,48 @@ namespace XenAdminTests { } - public ToolStripMenuItem ToolsMenu - { - get - { - return GetField("toolsToolStripMenuItem"); - } - } + public ToolStripMenuItem ToolsMenu => GetField("toolsToolStripMenuItem"); - public ToolsMenuWrapper ToolsMenuItems - { - get - { - return new ToolsMenuWrapper(Item); - } - } + public ToolsMenuWrapper ToolsMenuItems => new ToolsMenuWrapper(Item); - public ToolStripMenuItem WindowMenu - { - get - { - return GetField("windowToolStripMenuItem"); - } - } + public ToolStripMenuItem HelpMenu => GetField("helpToolStripMenuItem"); - public ToolStripMenuItem HelpMenu - { - get - { - return GetField("helpToolStripMenuItem"); - } - } + public ToolStripMenuItem ViewMenu => GetField("viewToolStripMenuItem"); - public ToolStripMenuItem ViewMenu - { - get - { - return GetField("viewToolStripMenuItem"); - } - } + public ToolStripMenuItem FileMenu => GetField("fileToolStripMenuItem"); + public ToolStripMenuItem PoolMenu => GetField("poolToolStripMenuItem"); - public ToolStripMenuItem FileMenu - { - get - { - return GetField("fileToolStripMenuItem"); - } - } + public ToolStripMenuItem VMMenu => GetField("VMToolStripMenuItem"); - public ToolStripMenuItem PoolMenu - { - get - { - return GetField("poolToolStripMenuItem"); - } - } + public ToolStripMenuItem TemplatesMenu => GetField("templatesToolStripMenuItem"); - public ToolStripMenuItem VMMenu - { - get - { - return GetField("VMToolStripMenuItem"); - } - } + public ToolStripEx MainToolStrip => GetField("ToolStrip"); - public ToolStripMenuItem TemplatesMenu - { - get - { - return GetField("templatesToolStripMenuItem"); - } - } + public MainToolBarWrapper MainToolStripItems => new MainToolBarWrapper(Item); - public ToolStripEx MainToolStrip - { - get - { - return GetField("ToolStrip"); - } - } + public PluginManager PluginManager => GetField("pluginManager"); - public MainToolBarWrapper MainToolStripItems - { - get - { - return new MainToolBarWrapper(Item); - } - } + public FlickerFreeTreeView TreeView => TestUtils.GetFlickerFreeTreeView(Item, "navigationPane.navigationView.treeView"); - public PluginManager PluginManager - { - get - { - return GetField("pluginManager"); - } - } + public ToolStripMenuItem StorageMenu => Item.StorageToolStripMenuItem; - public FlickerFreeTreeView TreeView - { - get { return TestUtils.GetFlickerFreeTreeView(Item, "navigationPane.navigationView.treeView"); } } + public StorageMenuWrapper StorageMenuItems => new StorageMenuWrapper(Item); - public CommandToolStripMenuItem AddHostToolStripMenuItemInPoolMenu - { - get - { - return GetField("addServerToolStripMenuItem"); - } - } + public ToolStripMenuItem HostMenu => GetField("HostMenuItem"); - public ToolStripMenuItem StorageMenu - { - get - { - return Item.StorageToolStripMenuItem; - } - } + public VMMenuWrapper VMMenuItems => new VMMenuWrapper(Item); - public StorageMenuWrapper StorageMenuItems - { - get - { - return new StorageMenuWrapper(Item); - } - } + public ViewMenuWrapper ViewMenuItems => new ViewMenuWrapper(Item); - public ToolStripMenuItem HostMenu - { - get - { - return GetField("HostMenuItem"); - } - } + public TabControl TheTabControl => Item.TheTabControl; - public HostMenuWrapper HostMenuItems - { - get - { - return new HostMenuWrapper(Item); - } - } + public MenuStripEx MainMenuBar => GetField("MainMenuBar"); - public VMMenuWrapper VMMenuItems - { - get - { - return new VMMenuWrapper(Item); - } - } + public NetworkTabPageWrapper NetworkPage => new NetworkTabPageWrapper(Item.NetworkPage); - public ViewMenuWrapper ViewMenuItems - { - get - { - return new ViewMenuWrapper(Item); - } - } + public TabPage TabPageNetwork => GetField("TabPageNetwork"); - public TabControl TheTabControl - { - get - { - return Item.TheTabControl; - } - } - - public MenuStripEx MainMenuBar - { - get - { - return GetField("MainMenuBar"); - } - } - - public NetworkTabPageWrapper NetworkPage - { - get - { - return new NetworkTabPageWrapper(Item.NetworkPage); - } - } - - public TabPage TabPageNetwork - { - get - { - return GetField("TabPageNetwork"); - } - } + public Form[] OwnedForms => Item.OwnedForms; } } diff --git a/XenAdminTests/MainWindowWrapper/TestWrapper.cs b/XenAdminTests/MainWindowWrapper/TestWrapper.cs index a2539aa2b..806f82f66 100644 --- a/XenAdminTests/MainWindowWrapper/TestWrapper.cs +++ b/XenAdminTests/MainWindowWrapper/TestWrapper.cs @@ -30,12 +30,10 @@ */ using System; -using System.Data; using System.Reflection; using System.Windows.Forms; using NUnit.Framework; using XenAdmin; -using XenAdmin.Core; namespace XenAdminTests { @@ -50,44 +48,25 @@ namespace XenAdminTests { private readonly TClass _item; - /// - /// Initializes a new instance of the class. - /// - /// The class that is to be wrapped. - public TestWrapper(TClass item) + protected TestWrapper(TClass item) { Util.ThrowIfParameterNull(item, "item"); _item = item; } - private static TClass GetControlFromWindow(IWin32Window window) - { - Control control = Control.FromHandle(window.Handle); - - if (!(control is TClass)) - { - throw new ArgumentException("window is not a " + typeof(TClass).Name, "window"); - } - - return control as TClass; - } - - /// - /// Initializes a new instance of class. - /// - /// The window. - public TestWrapper(IWin32Window window) + protected TestWrapper(IWin32Window window) : this(GetControlFromWindow(window)) { } - /// - /// Initializes a new instance of the class. - /// - /// The window text of the window being wrapped. - public TestWrapper(string windowText) - : this(Win32Window.GetWindowWithText(windowText, w => Control.FromHandle(w.Handle) is TClass)) + private static TClass GetControlFromWindow(IWin32Window window) { + var control = Control.FromHandle(window.Handle) as TClass; + + if (control == null) + throw new ArgumentException($"window is not a {typeof(TClass).Name} window"); + + return control; } /// @@ -112,10 +91,10 @@ namespace XenAdminTests } /// - /// Gets the value of the private field from the wrapped classes *base* class of the specified name. + /// Gets the value of a given private field from the wrapped class's *base* class /// /// The type of the field. - /// The name of the private field. + /// The name of the field. /// The value of the private field from the base class of the wrapped class of the specified name. protected TField GetBaseClassField(string name) { @@ -125,14 +104,13 @@ namespace XenAdminTests { Type baseType = _item.GetType().BaseType; - if(baseType == null) - throw new NoNullAllowedException("Base class type was null, check the class has a base class"); + Assert.NotNull(baseType, $"{name} is a field of a class that has no base class"); return (TField)baseType.GetField(name, BindingFlags.NonPublic | BindingFlags.Instance).GetValue(_item); } catch (Exception e) { - Assert.Fail(string.Format("Field {0} of {1} throws {2}.", name, typeof(TClass).Name, e.GetType().Name)); + Assert.Fail($"Field {name} of {typeof(TClass).Name} throws {e.GetType().Name}."); throw; } } diff --git a/XenAdminTests/MiscTests/WebBrowser2Tests.cs b/XenAdminTests/MiscTests/WebBrowser2Tests.cs index f74700b40..3905de083 100644 --- a/XenAdminTests/MiscTests/WebBrowser2Tests.cs +++ b/XenAdminTests/MiscTests/WebBrowser2Tests.cs @@ -118,37 +118,21 @@ namespace XenAdminTests.MiscTests [Test] public void TestMultipleUrisWithNoneValid() { - Uri uri1 = new Uri("http://fgdfffgd.dfgdfgd.dfg"); - Uri uri2 = new Uri("http://fgdfgd.dfgdfgd.dfg"); - Uri navCancelUri = new Uri("res://ieframe.dll/navcancl.htm#http://fgdfgd.dfgdfgd.dfg/"); + Uri curUri = null; bool navigating = false; bool navigated = false; bool navError = false; _wb.Navigating += (s, e) => { - if (e.Url != navCancelUri) - { - Assert.IsFalse(navigating); - } - else - { - Assert.IsTrue(navigating); - } - navigating = true; + if (e.Url == curUri) + navigating = true; }; _wb.Navigated += (s, e) => { - if (e.Url != navCancelUri) - { - Assert.IsFalse(navigated); - } - else - { - Assert.IsTrue(navigated); - } - navigated = true; + if (e.Url == curUri) + navigated = true; }; _wb.NavigateError += (s, e) => @@ -157,10 +141,20 @@ namespace XenAdminTests.MiscTests navError = true; }; - var uris = new[] { uri1, uri2 }; + //Use non-existing URLs with existing rather than completely fictional + //domains, otherwise the NavigationError even is not fired + + var uris = new[] + { + new Uri("http://127.0.0.1/blah"), + new Uri("http://127.0.0.1/blahblah") + }; + foreach (var uri in uris) { - var curUri = uri; + curUri = uri; + + navigated = navigated = navError = false; MW(() => _wb.Navigate(curUri)); MWWaitFor(() => navigating && navigated && navError, "Navigation didn't take place."); }