fix installation crash from StaticBoxSizer changes (nvaccess#12199)

Summary of the issue:
The standard installation process crashes with the following traceback, caused by nvaccess#12181

Description of how this pull request fixes the issue:
Fix various crashing GUI bugs introduced to the installation process in nvaccess#12181
Add smoke tests for the installation process

appveyor.yml

@@ -133,6 +133,7 @@ before_test:
       $nvdaLauncherFile+="_snapshot"
      }
      $nvdaLauncherFile+="_${env:version}.exe"
+     Set-AppveyorBuildVariable "nvdaLauncherFile" $nvdaLauncherFile
      echo NVDALauncherFile: $NVDALauncherFile
      $outputDir=$(resolve-path .\testOutput)
      $installerLogFilePath="$outputDir\nvda_install.log"
@@ -194,8 +195,7 @@ test_script:
  - ps: |
      $testOutput = (Resolve-Path .\testOutput\)
      $systemTestOutput = (Resolve-Path "$testOutput\system")
-    
-     .\runsystemtests.bat --variable whichNVDA:installed
+     .\runsystemtests.bat --variable whichNVDA:installed --variable installDir:"${env:nvdaLauncherFile}" --include installer
      if($LastExitCode -ne 0) {
       $errorCode=$LastExitCode
       Add-AppveyorMessage "System test failure"

source/gui/installerGui.py

@@ -177,13 +177,13 @@ def __init__(self, parent, isUpdate):
 
 		# Translators: The label for a group box containing the NVDA installation dialog options.
 		optionsLabel = _("Options")
-		optionsHelper = sHelper.addItem(wx.StaticBoxSizer(wx.VERTICAL, self, label=optionsLabel))
-		optionsSizer = guiHelper.BoxSizerHelper(self, sizer=optionsHelper)
+		optionsSizer = sHelper.addItem(wx.StaticBoxSizer(wx.VERTICAL, self, label=optionsLabel))
+		optionsHelper = guiHelper.BoxSizerHelper(self, sizer=optionsSizer)
 		optionsBox = optionsSizer.GetStaticBox()
 
 		# Translators: The label of a checkbox option in the Install NVDA dialog.
 		startOnLogonText = _("Use NVDA during sign-in")
-		self.startOnLogonCheckbox = optionsSizer.addItem(wx.CheckBox(optionsBox, label=startOnLogonText))
+		self.startOnLogonCheckbox = optionsHelper.addItem(wx.CheckBox(optionsBox, label=startOnLogonText))
 		self.bindHelpEvent("StartAtWindowsLogon", self.startOnLogonCheckbox)
 		if globalVars.appArgs.enableStartOnLogon is not None:
 			self.startOnLogonCheckbox.Value = globalVars.appArgs.enableStartOnLogon
@@ -195,21 +195,21 @@ def __init__(self, parent, isUpdate):
 			# Translators: The label of a checkbox option in the Install NVDA dialog.
 			keepShortCutText = _("&Keep existing desktop shortcut")
 			keepShortCutBox = wx.CheckBox(optionsBox, label=keepShortCutText)
-			self.createDesktopShortcutCheckbox = optionsSizer.addItem(keepShortCutBox)
+			self.createDesktopShortcutCheckbox = optionsHelper.addItem(keepShortCutBox)
 		else:
 			# Translators: The label of the option to create a desktop shortcut in the Install NVDA dialog.
 			# If the shortcut key has been changed for this locale,
 			# this change must also be reflected here.
 			createShortcutText = _("Create &desktop icon and shortcut key (control+alt+n)")
 			createShortcutBox = wx.CheckBox(optionsBox, label=createShortcutText)
-			self.createDesktopShortcutCheckbox = optionsSizer.addItem(createShortcutBox)
+			self.createDesktopShortcutCheckbox = optionsHelper.addItem(createShortcutBox)
 		self.bindHelpEvent("CreateDesktopShortcut", self.createDesktopShortcutCheckbox)
 		self.createDesktopShortcutCheckbox.Value = shortcutIsPrevInstalled if self.isUpdate else True 
 
 		# Translators: The label of a checkbox option in the Install NVDA dialog.
 		createPortableText = _("Copy &portable configuration to current user account")
 		createPortableBox = wx.CheckBox(optionsBox, label=createPortableText)
-		self.copyPortableConfigCheckbox = optionsSizer.addItem(createPortableBox)
+		self.copyPortableConfigCheckbox = optionsHelper.addItem(createPortableBox)
 		self.bindHelpEvent("CopyPortableConfigurationToCurrentUserAccount", self.copyPortableConfigCheckbox)
 		self.copyPortableConfigCheckbox.Value = bool(globalVars.appArgs.copyPortableConfig)
 		if globalVars.appArgs.copyPortableConfig is None:
@@ -220,12 +220,12 @@ def __init__(self, parent, isUpdate):
 		bHelper = sHelper.addDialogDismissButtons(guiHelper.ButtonHelper(wx.HORIZONTAL))
 		if shouldAskAboutAddons:
 			# Translators: The label of a button to launch the add-on compatibility review dialog.
-			reviewAddonButton = bHelper.addButton(optionsBox, label=_("&Review add-ons..."))
+			reviewAddonButton = bHelper.addButton(self, label=_("&Review add-ons..."))
 			self.bindHelpEvent("InstallWithIncompatibleAddons", reviewAddonButton)
 			reviewAddonButton.Bind(wx.EVT_BUTTON, self.onReviewAddons)
 
 		# Translators: The label of a button to continue with the operation.
-		continueButton = bHelper.addButton(optionsBox, label=_("&Continue"), id=wx.ID_OK)
+		continueButton = bHelper.addButton(self, label=_("&Continue"), id=wx.ID_OK)
 		continueButton.SetDefault()
 		continueButton.Bind(wx.EVT_BUTTON, self.onInstall)
 		if shouldAskAboutAddons:
@@ -235,7 +235,7 @@ def __init__(self, parent, isUpdate):
 			)
 			continueButton.Enable(False)
 
-		bHelper.addButton(optionsBox, id=wx.ID_CANCEL)
+		bHelper.addButton(self, id=wx.ID_CANCEL)
 		# If we bind this using button.Bind, it fails to trigger when the dialog is closed.
 		self.Bind(wx.EVT_BUTTON, self.onCancel, id=wx.ID_CANCEL)
 
@@ -365,22 +365,22 @@ def __init__(self, parent):
 
 		# Translators: The label of a checkbox option in the Create Portable NVDA dialog.
 		copyConfText = _("Copy current &user configuration")
-		self.copyUserConfigCheckbox = sHelper.addItem(wx.CheckBox(groupBox, label=copyConfText))
+		self.copyUserConfigCheckbox = sHelper.addItem(wx.CheckBox(self, label=copyConfText))
 		self.copyUserConfigCheckbox.Value = False
 		if globalVars.appArgs.launcher:
 			self.copyUserConfigCheckbox.Disable()
 		# Translators: The label of a checkbox option in the Create Portable NVDA dialog.
 		startAfterCreateText = _("&Start the new portable copy after creation")
-		self.startAfterCreateCheckbox = sHelper.addItem(wx.CheckBox(groupBox, label=startAfterCreateText))
+		self.startAfterCreateCheckbox = sHelper.addItem(wx.CheckBox(self, label=startAfterCreateText))
 		self.startAfterCreateCheckbox.Value = False
 
 		bHelper = sHelper.addDialogDismissButtons(gui.guiHelper.ButtonHelper(wx.HORIZONTAL), separated=True)
 
-		continueButton = bHelper.addButton(groupBox, label=_("&Continue"), id=wx.ID_OK)
+		continueButton = bHelper.addButton(self, label=_("&Continue"), id=wx.ID_OK)
 		continueButton.SetDefault()
 		continueButton.Bind(wx.EVT_BUTTON, self.onCreatePortable)
 
-		bHelper.addButton(groupBox, id=wx.ID_CANCEL)
+		bHelper.addButton(self, id=wx.ID_CANCEL)
 		# If we bind this using button.Bind, it fails to trigger when the dialog is closed.
 		self.Bind(wx.EVT_BUTTON, self.onCancel, id=wx.ID_CANCEL)
 

source/gui/startupDialogs.py

@@ -131,7 +131,7 @@ class LauncherDialog(
 	helpId = "InstallingNVDA"
 
 	def __init__(self, parent):
-		super().__init__(parent, title=versionInfo.name)
+		super().__init__(parent, title=f"{versionInfo.name} {_('Launcher')}")
 
 		mainSizer = wx.BoxSizer(wx.VERTICAL)
 		sHelper = gui.guiHelper.BoxSizerHelper(self, orientation=wx.VERTICAL)

tests/system/libraries/NvdaLib.py

@@ -49,12 +49,16 @@ def __init__(self):
 		opSys.directory_should_exist(self.stagingDir)
 
 		self.whichNVDA = builtIn.get_variable_value("${whichNVDA}", "source")
+		self._installFilePath = builtIn.get_variable_value("${installDir}", None)
+		self.NVDAInstallerCommandline = None
 		if self.whichNVDA == "source":
 			self._runNVDAFilePath = _pJoin(self.repoRoot, "runnvda.bat")
 			self.baseNVDACommandline = self._runNVDAFilePath
 		elif self.whichNVDA == "installed":
 			self._runNVDAFilePath = _pJoin(_expandvars('%PROGRAMFILES%'), 'nvda', 'nvda.exe')
 			self.baseNVDACommandline = f'"{str(self._runNVDAFilePath)}"'
+			if self._installFilePath is not None:
+				self.NVDAInstallerCommandline = f'"{str(self._installFilePath)}"'
 		else:
 			raise AssertionError("RobotFramework should be run with argument: '-v whichNVDA [source|installed]'")
 
@@ -65,8 +69,15 @@ def __init__(self):
 			"nvdaTestRunLogs"
 		)
 
+	def ensureInstallerPathsExist(self):
+		fileWarnMsg = f"Unable to run NVDA installer unless path exists. Path given: {self._installFilePath}"
+		opSys.file_should_exist(self._installFilePath, fileWarnMsg)
+		opSys.create_directory(self.profileDir)
+		opSys.create_directory(self.preservedLogsDir)
+
 	def ensurePathsExist(self):
-		opSys.file_should_exist(self._runNVDAFilePath, "Unable to start NVDA unless path exists.")
+		fileWarnMsg = f"Unable to run NVDA installer unless path exists. Path given: {self._runNVDAFilePath}"
+		opSys.file_should_exist(self._runNVDAFilePath, fileWarnMsg)
 		opSys.create_directory(self.profileDir)
 		opSys.create_directory(self.preservedLogsDir)
 
@@ -130,7 +141,28 @@ def _startNVDAProcess(self):
 		)
 		return handle
 
-	def _connectToRemoteServer(self):
+	def _startNVDAInstallerProcess(self):
+		"""Start NVDA Installer.
+		Use debug logging, replacing any current instance, using the system test profile directory
+		"""
+		_locations.ensureInstallerPathsExist()
+		command = (
+			f"{_locations.NVDAInstallerCommandline}"
+			f" --debug-logging"
+			f" -r"
+			f" -c \"{_locations.profileDir}\""
+			f" --log-file \"{_locations.logPath}\""
+		)
+		self.nvdaHandle = handle = process.start_process(
+			command,
+			shell=True,
+			alias=self.nvdaProcessAlias,
+			stdout=_pJoin(_locations.preservedLogsDir, self._createTestIdFileName("stdout.txt")),
+			stderr=_pJoin(_locations.preservedLogsDir, self._createTestIdFileName("stderr.txt")),
+		)
+		return handle
+
+	def _connectToRemoteServer(self, connectionTimeoutSecs=10):
 		"""Connects to the nvdaSpyServer
 		Because we do not know how far through the startup NVDA is, we have to poll
 		to check that the server is available. Importing the library immediately seems
@@ -146,7 +178,7 @@ def _connectToRemoteServer(self):
 		# therefore we use '_testRemoteServer' to ensure that we can in fact connect before proceeding.
 		_blockUntilConditionMet(
 			getValue=lambda: _testRemoteServer(self._spyServerURI, log=False),
-			giveUpAfterSeconds=10,
+			giveUpAfterSeconds=connectionTimeoutSecs,
 			errorMessage=f"Unable to connect to {self._spyAlias}",
 		)
 		builtIn.log(f"Connecting to {self._spyAlias}", level='DEBUG')
@@ -193,6 +225,16 @@ def runKeyword(*args, **kwargs):
 			)
 		return remoteLib
 
+	def start_NVDAInstaller(self, settingsFileName):
+		builtIn.log(f"Starting NVDA with config: {settingsFileName}")
+		self.setup_nvda_profile(settingsFileName)
+		nvdaProcessHandle = self._startNVDAInstallerProcess()
+		process.process_should_be_running(nvdaProcessHandle)
+		# Timeout is increased due to the installer load time and start up splash sound
+		self._connectToRemoteServer(connectionTimeoutSecs=30)
+		self.nvdaSpy.wait_for_NVDA_startup_to_complete()
+		return nvdaProcessHandle
+
 	def start_NVDA(self, settingsFileName):
 		builtIn.log(f"Starting NVDA with config: {settingsFileName}")
 		self.setup_nvda_profile(settingsFileName)
@@ -234,6 +276,22 @@ def quit_NVDA(self):
 			# remove the spy so that if nvda is run manually against this config it does not interfere.
 			self.teardown_nvda_profile()
 
+	def quit_NVDAInstaller(self):
+		builtIn.log("Stopping nvdaSpy server: {}".format(self._spyServerURI))
+		self.nvdaSpy.emulateKeyPress("insert+q")
+		self.nvdaSpy.wait_for_specific_speech("Exit NVDA")
+		self.nvdaSpy.emulateKeyPress("enter", blockUntilProcessed=False)
+		builtIn.sleep(1)
+		try:
+			_stopRemoteServer(self._spyServerURI, log=False)
+		except Exception:
+			raise
+		finally:
+			self.save_NVDA_log()
+			# remove the spy so that if nvda is run manually against this config it does not interfere.
+			self.teardown_nvda_profile()
+
+
 
 def getSpyLib():
 	""" Gets the spy library instance. This has been augmented with methods for all supported keywords.

tests/system/readme.md

@@ -55,6 +55,9 @@ of NVDA (first ensure it is compatible with the tests). Note valid values are:
 the tests are run from an administrator command prompt.
 * "source"
 
+The `installDir` argument performs a smoke test on the installation process given a path to the installer exe. For example `--variable installDir:".\path\to\nvda_installer.exe"`.
+This should be used with `--variable whichNVDA:installed --include installer`.
+
 ### Overview
 
 Robot Framework loads and parses the test files and their libraries.

tests/system/robot/NVDAInstaller.py

@@ -0,0 +1,70 @@
+# A part of NonVisual Desktop Access (NVDA)
+# Copyright (C) 2020-2021 NV Access Limited
+# This file may be used under the terms of the GNU General Public License, version 2 or later.
+# For more details see: https://www.gnu.org/licenses/gpl-2.0.html
+
+"""Logic for NVDA installation process tests.
+"""
+
+from robot.libraries.BuiltIn import BuiltIn
+# relative import not used for 'systemTestUtils' because the folder is added to the path for 'libraries'
+# imported methods start with underscore (_) so they don't get imported into robot files as keywords
+from SystemTestSpy import (
+	_getLib,
+)
+
+# Imported for type information
+from robot.libraries.Process import Process as _ProcessLib
+
+from AssertsLib import AssertsLib as _AssertsLib
+
+import NvdaLib as _nvdaLib
+from NvdaLib import NvdaLib as _nvdaRobotLib
+_nvdaProcessAlias = _nvdaRobotLib.nvdaProcessAlias
+
+_builtIn: BuiltIn = BuiltIn()
+_process: _ProcessLib = _getLib("Process")
+_asserts: _AssertsLib = _getLib("AssertsLib")
+
+
+def read_install_dialog():
+	"Smoke test the launcher dialogs used to install NVDA"
+
+	spy = _nvdaLib.getSpyLib()
+	launchDialog = spy.wait_for_specific_speech("NVDA Launcher")  # ensure the dialog is present.
+	spy.wait_for_speech_to_finish()
+	spy.get_speech_at_index_until_now(launchDialog)
+
+	_builtIn.sleep(1)  # the dialog is not always receiving keypresses, wait a little longer for it
+	# agree to the License Agreement
+	spy.emulateKeyPress("alt+a")
+
+	# start install
+	spy.emulateKeyPress("alt+i")
+
+	spy.wait_for_specific_speech("To install NVDA to your hard drive, please press the Continue button.")
+
+	# exit NVDA Installer
+	spy.emulateKeyPress("escape")
+
+
+def read_portable_copy_dialog():
+	"Smoke test the launcher dialogs used to create a portable copy of NVDA"
+
+	spy = _nvdaLib.getSpyLib()
+	launchDialog = spy.wait_for_specific_speech("NVDA Launcher")  # ensure the dialog is present.
+	spy.wait_for_speech_to_finish()
+	spy.get_speech_at_index_until_now(launchDialog)
+
+	_builtIn.sleep(1)  # the dialog is not always receiving keypresses, wait a little longer for it
+	# agree to the License Agreement
+	spy.emulateKeyPress("alt+a")
+
+	# start portable copy
+	spy.emulateKeyPress("alt+p")
+
+	spy.wait_for_specific_speech(
+		"To create a portable copy of NVDA, please select the path and other options and then press Continue")
+
+	# exit NVDA Installer
+	spy.emulateKeyPress("escape")

tests/system/robot/NVDAInstaller.robot

@@ -0,0 +1,38 @@
+# A part of NonVisual Desktop Access (NVDA)
+# Copyright (C) 2021 NV Access Limited
+# This file may be used under the terms of the GNU General Public License, version 2 or later.
+# For more details see: https://www.gnu.org/licenses/gpl-2.0.html
+*** Settings ***
+Documentation	Smoke test the installation process
+Force Tags	smoke test	installer
+Suite Setup	Run Keyword If	not r'${installDir}'	Fail	"--installDir not supplied"
+
+# for start & quit in Test Setup and Test Teardown
+Library	NvdaLib.py
+Library	NVDAInstaller.py
+Library	ScreenCapLibrary
+
+Test Setup	default startup
+Test Teardown	default teardown
+
+*** Keywords ***
+
+default teardown
+	${screenshotName}=	create_preserved_test_output_filename	failedTest.png
+	Run Keyword If Test Failed	Take Screenshot	${screenShotName}
+	quit NVDAInstaller
+
+default startup
+	start NVDAInstaller	standard-dontShowWelcomeDialog.ini
+
+default pass execution
+
+*** Test Cases ***
+
+Read install dialog
+	[Documentation]	Ensure that the install dialog can be read in full
+	read_install_dialog	# run test
+
+Read install dialog portable copy
+	[Documentation]	Ensure that the portable copy install dialog can be read in full
+	read_portable_copy_dialog	# run test

tests/system/robotArgs.robot

@@ -2,5 +2,7 @@
 --outputdir testOutput\system
 --xunit systemTests.xml
 --pythonpath .\tests\system\libraries
+--include NVDA
 --exclude excluded_from_build
 --variable whichNVDA:source
+--variable installDir: