FWD » Core Development » Base Language

I'm extracting that from another unrelated issue just so it doesn't get lost.

Vladimir, we're running ABLUnit from Eclipse and you're right as it doesn't seems to be any option for pass any configuration parameters when the engine is initialized... what you could do maybe is to allow usage of an environment variable (fwd_config_file) and use that instead of the default if set in FWDTestEngine.

String configFile = configurationParameters.get(CFG_FWD_CONFIG_FILE).orElse(System.getenv(CFG_FWD_CONFIG_FILE));

if (initialize(configFile != null ? configFile : DEFAULT_FWD_CONFIG_FILE)) {

The reason behind this is the use of relative paths in PROPATH makes the execution dependent on the current working directory hence running it from client_gui/client_chui so it can pick-up the right abl_unit.xml file is causing issues on finding resources using relative path.

Thanks

IMO this will add another layer of complexity and mixed technology. Besides, using OS environment variables with Java is not recommended.

I would prefer if the FWD client configuration was implemented with Java properties instead of command-line arguments, which can be used with Eclipse.

The command line may look like:

-Dnet.server.host=localhost -Dnet.connection.secure=false -Dnet.server.secure_port=4434 -Dnet.server.insecure_port=4534 -Dclient.driver.type=gui_swing -Dclient.driver.theme=com.goldencode.p2j.ui.client.gui.theme.Windows10Theme

And standard Java property files can be used in place of XML files.

Vladimir Tsichevski wrote:

IMO this will add another layer of complexity and mixed technology. Besides, using OS environment variables with Java is not recommended.

I would prefer if the FWD client configuration was implemented with Java properties instead of command-line arguments, which can be used with Eclipse.

The command line may look like:

[...]

And standard Java property files can be used in place of XML files.

I really have no preference here, whatever you think is appropriate, just not force the location of unit_test.xml file to be in current folder :)

We can certainly send command line arguments to the JUnit run-configuration, only I don't know exactly where you can extract those so environment variables was the easiest way for me, recommended or not those are there and can certainly be used :)

Marian Edu wrote:

Vladimir Tsichevski wrote:

IMO this will add another layer of complexity and mixed technology. Besides, using OS environment variables with Java is not recommended.

I would prefer if the FWD client configuration was implemented with Java properties instead of command-line arguments, which can be used with Eclipse.

The command line may look like:

[...]

And standard Java property files can be used in place of XML files.

I really have no preference here, whatever you think is appropriate, just not force the location of unit_test.xml file to be in current folder :)

We can certainly send command line arguments to the JUnit run-configuration, only I don't know exactly where you can extract those so environment variables was the easiest way for me, recommended or not those are there and can certainly be used :)

Using Java properties for client configuration will allow to run with no configuration files, so we will not have to bother about the client process working directory.

Vladimir Tsichevski wrote:

Using Java properties for client configuration will allow to run with no configuration files, so we will not have to bother about the client process working directory.

I would rather not send all client properties that way, and I expect the use of XML configuration instead of plain properties files has something to do with the hierarchical structure that is needed - maybe not for the client though, I do not know, but any way it looks like it's just reusing the same bootstrap mechanism so really the easiest way for us at this point is to make one option to set the unit_test.xml (actually client configuration) when running from Eclipse. If not through environment variables I'm ok with a command argument but is not my call to make a design decision here :)

For time being we will just run with our local tweak based on environment variables and will switch to whatever needed when something else is available.

Thanks

I agree, we should add the ability to configure the bootstrap cfg file name (and only the bootstrap cfg file name) via Java property.

I don't want to add any other property support, as this would duplicate what we already have for the bootstrap cfg.

Marian Edu wrote:

This is a patch with the local changes we are using

Do you mind if I create the 8138a branch and apply your patch there to make the further discussion easier?

Yes, good idea.

Greg Shah wrote:

Yes, good idea.

Done in 8138a rev. 15628. The patch from Marian applied, code indentation, style and formatting fixed.

Greg Shah wrote:

This still needs to be discussed and resolved.

I will look at it when I get back at the office on 9th, my assumption was the 'shutdown' of any screen driver will do whatever clean-up needed.

Greg Shah wrote:

Code Review testengine.patch

Doesn't the change to use the driver's shutdown() bypass the ncurses terminate() call since it is not Windows? ConsoleDriver.shutdown() only calls terminate() if PlatformHelper.isUnderWindowsFamily() is true. That seems like a problem.

Looking at the code now the shutdown method isn't used nor it was used from ConsoleDriver, only suspend and resume methods were called there. As far as I can see if the screen driver used is ConsoleDriver the suspend/resume methods will proxy those to the ConsoleHelper so it's exactly the same behaviour as before. If the driver isn't the ConsoleDriver does the ConsoleHelper need to be called at all? In our environment under windows the code as it was before lead to a GPF in native code.

Vladimir: I think you should take it from here.

Greg Shah wrote:

Vladimir: I think you should take it from here.

You mean review, test and merge?

I mean address/analyze/discuss this:

Doesn't the change to use the driver's shutdown() bypass the ncurses terminate() call since it is not Windows? ConsoleDriver.shutdown() only calls terminate() if PlatformHelper.isUnderWindowsFamily() is true. That seems like a problem.

Looking at the code now the shutdown method isn't used nor it was used from ConsoleDriver, only suspend and resume methods were called there. As far as I can see if the screen driver used is ConsoleDriver the suspend/resume methods will proxy those to the ConsoleHelper so it's exactly the same behaviour as before. If the driver isn't the ConsoleDriver does the ConsoleHelper need to be called at all? In our environment under windows the code as it was before lead to a GPF in native code.

Greg Shah wrote:

I mean address/analyze/discuss this:

Doesn't the change to use the driver's shutdown() bypass the ncurses terminate() call since it is not Windows? ConsoleDriver.shutdown() only calls terminate() if PlatformHelper.isUnderWindowsFamily() is true. That seems like a problem.

Looking at the code now the shutdown method isn't used nor it was used from ConsoleDriver, only suspend and resume methods were called there. As far as I can see if the screen driver used is ConsoleDriver the suspend/resume methods will proxy those to the ConsoleHelper so it's exactly the same behaviour as before. If the driver isn't the ConsoleDriver does the ConsoleHelper need to be called at all? In our environment under windows the code as it was before lead to a GPF in native code.

OK, note I have no Windows environment, so I can mainly test/analyze this on Linux.

Is this task active?

My suggestion was in #8138-7.

I was not able to rebase 8138a, and created a new branch 8138b.

8131b r.16294: Introduce Java system property for specifying the ABLUnit bootstrap XML configuration file replacing the OS environment variable approach, as agreed in #8138-7.

Tested ABLUnit test execution on branch 8138b under Linux (both via Eclipse and console): no issues observed.

To validate test engine initialization failure handling, I emulated test engine startup errors by:

• Inducing a NPE on the server side using remote debugger.
• Launching the client without a server available for connection.

Testing covered modes: swing_gui and chui_native in batch and nonbatch.

This confirmed correct behavior in the code, where the explicit call to ConsoleHelper.terminate() has been replaced by is.driver.shutdown().
On Linux, the explicit ConsoleHelper.terminate() call is redundant, as it is already managed via atexit in terminal_linux.c.

Rebased branch 8138b to trunk revision 16301. Please, review.