Should NRunner support the AVOCADO_* environment variables for non-instrumented tests like the legacy runner?

[willianrampazzo]

While I was investigating how to implement #4742, I reached some walls that depend on some discussion.

In the legacy runner, the following environment variables are set for any kind of test:

• AVOCADO_VERSION;
• AVOCADO_TEST_BASEDIR;
• AVOCADO_TEST_WORKDIR;
• AVOCADO_TEST_LOGDIR;
• AVOCADO_TEST_LOGFILE;
• AVOCADO_TEST_OUTPUTDIR.

For instrumented tests, those environment variables are set inside the Test class, independent of runner/nrunner:

avocado/avocado/core/test.py

Lines 874 to 883 in 8c87bfe

	def _setup_environment_variables(self):
	os.environ['AVOCADO_VERSION'] = VERSION
	if self.basedir is not None:
	os.environ['AVOCADO_TEST_BASEDIR'] = self.basedir
	os.environ['AVOCADO_TEST_WORKDIR'] = self.workdir
	os.environ['AVOCADO_TEST_LOGDIR'] = self.logdir
	os.environ['AVOCADO_TEST_LOGFILE'] = self.logfile
	os.environ['AVOCADO_TEST_OUTPUTDIR'] = self.outputdir
	if self.__sysinfo_enabled:
	os.environ['AVOCADO_TEST_SYSINFODIR'] = self.__sysinfodir

For non-instrumented tests, while it is possible to set AVOCADO_VERSION, AVOCADO_TEST_BASEDIR, AVOCADO_TEST_LOGDIR, and AVOCADO_TEST_OUTPUTDIR using the values from runnable config, AVOCADO_TEST_WORKDIR and AVOCADO_TEST_LOGFILE are not defined.

In the current implementation of nrunner, the information about the test is exchanged using the messages mechanism, so the AVOCADO_TEST_LOGFILE does not need to be available to the test or even exist in some cases.

The AVOCADO_TEST_WORKDIR is a temporary folder created for the test. It can make use of the folder if needed.

Below is a search restricted to GitHub of projects using those environment variables:

• AVOCADO_VERSION

  • https://github.com/dmonakhov/debian-avocado/blob/fce2e8245bf8311d969f92cc7b6d0569970f35b5/update.sh

• AVOCADO_TEST_BASEDIR

  • No occurrences

• AVOCADO_TEST_WORKDIR

  • No valid occurrences (there is one occurrence from an old QEMU acceptance test code)

• AVOCADO_TEST_LOGDIR

  • No occurrences

• AVOCADO_TEST_LOGFILE

  • No occurrences

• AVOCADO_TEST_OUTPUTDIR

  • https://github.com/dmonakhov/fio-testsuite-generator/blob/4138a42db5d2b42e364cbd5a791b43333fd68870/templates/run_jobs.sh.j2
  • https://github.com/dmonakhov/vzt-ktest-util/blob/c4e0c8e79360caad351f6220a102688f7c0437b8/vzt-kernel-install
  • https://github.com/dmonakhov/jenkins-pipeline-tests/blob/3c37789af2b66090605921450c84e5c4e466b28a/avocado/xfstests-bld.sh
  • https://github.com/avocado-framework/avocado-misc-tests/blob/9f73c300f825666f544c37734eb660d4ac773759/io/net/ethtool/ethtool_tests.sh
  • https://github.com/chenjing236/avocado-cloudtest/blob/7c1c9790fc24017d9f0bce6462a7271a8ea731f0/cloudtest/openstack/compute.py

Considering users may not be using most of these environment variables, and given the dilemma of supporting some on nrunner, like AVOCADO_TEST_WORKDIR and AVOCADO_TEST_LOGFILE, the questions I have to start a discussion are:

• Should we support all of these environment variables for non-instrumented tests on nrunner?
• Should we support only those already available for non-instrumented tests on runnable config?
• Should we deprecate the use of those environment variables for non-instrumented tests on nrunner?

[beraldoleal]

Hi @willianrampazzo, sorry for the delay here and thanks for bring starting this discussion.

IMO, we should think about some method of sharing those with the all type of tests, but not only those variables. Besides the first one (AVOCADO_VERSION), IIUC, most of those variables are present inside settings.as_dict() dict (if not there they could be present).

Maybe we could save the entire runtime dict at some place and export only one env: AVOCADO_RUNNING_CONFIG pointing to this .json|yaml file. So we let test writers parse this for querying values.

What do you think?

[clebergnu]

Hi @willianrampazzo ,

First let me try to summarize your findings about each of the environment variables. First, the non issues are:

• AVOCADO_VERSION: https://github.com/dmonakhov/debian-avocado/blob/fce2e8245bf8311d969f92cc7b6d0569970f35b5/update.sh is not really a test, but a script to update the "fedora-avocado" container, previously used with the deprecated/removed "docker" runner plugin
• AVOCADO_TEST_BASEDIR: No occurrences
• AVOCADO_TEST_WORKDIR: No valid occurrences
• AVOCADO_TEST_LOGDIR: No occurrences
• AVOCADO_TEST_LOGFILE: No occurrences

Which leaves us with AVOCADO_TEST_OUTPUTDIR as the one variable that is used more often, correct?

Now, about your questions, I'm following up with my answers:

Q) Should we support all of these environment variables for non-instrumented tests on nrunner?
A) I don't think we should attempt to re-create all of the Avocado Python API and express it "generic" terms such as environment variables, I think we need to keep those to a bare minimum. There's a balance somewhere between the "reuse existing tests" and "rewrite your tests" that we must meet. This can be thought of in two ways: 1) if a test type is introduced for a new language, then its runner should make APIs available in a native way. 2) For exec-test and similar tests that simply get executed, environment variables are the best (and worst :) way to implement these "APIs", so we attempt to make them available.

Q) Should we support only those already available for non-instrumented tests on runnable config?
A) I think we should support all environment variables that do not cause an architectural issue. For instance, with the nrunner architecture, AVOCADO_VERSION is meaningless, because the runner itself may be written in a different language, installed from a different repo, etc. The only requirement for the runner is that it complies with the expected command line interface, and the message format. So, in the future, it'd make sense to have a AVOCADO_RUNNER_PROTOCOL_VERSION of sorts, but not an AVOCADO_VERSION.

Q) Should we deprecate the use of those environment variables for non-instrumented tests on nrunner?
A) I don't think we have an alternative for exec-test and related... deprecating or eliminating that completely would cause an unfavorable unbalance in the "reuse existing tests" side of the scale.

[willianrampazzo]

Hi @willianrampazzo ,

First let me try to summarize your findings about each of the environment variables. First, the non issues are:

• AVOCADO_VERSION: https://github.com/dmonakhov/debian-avocado/blob/fce2e8245bf8311d969f92cc7b6d0569970f35b5/update.sh is not really a test, but a script to update the "fedora-avocado" container, previously used with the deprecated/removed "docker" runner plugin
• AVOCADO_TEST_BASEDIR: No occurrences
• AVOCADO_TEST_WORKDIR: No valid occurrences
• AVOCADO_TEST_LOGDIR: No occurrences
• AVOCADO_TEST_LOGFILE: No occurrences

Which leaves us with AVOCADO_TEST_OUTPUTDIR as the one variable that is used more often, correct?

Now, about your questions, I'm following up with my answers:

Q) Should we support all of these environment variables for non-instrumented tests on nrunner?
A) I don't think we should attempt to re-create all of the Avocado Python API and express it "generic" terms such as environment variables, I think we need to keep those to a bare minimum. There's a balance somewhere between the "reuse existing tests" and "rewrite your tests" that we must meet. This can be thought of in two ways: 1) if a test type is introduced for a new language, then its runner should make APIs available in a native way. 2) For exec-test and similar tests that simply get executed, environment variables are the best (and worst :) way to implement these "APIs", so we attempt to make them available.

Q) Should we support only those already available for non-instrumented tests on runnable config?
A) I think we should support all environment variables that do not cause an architectural issue. For instance, with the nrunner architecture, AVOCADO_VERSION is meaningless, because the runner itself may be written in a different language, installed from a different repo, etc. The only requirement for the runner is that it complies with the expected command line interface, and the message format. So, in the future, it'd make sense to have a AVOCADO_RUNNER_PROTOCOL_VERSION of sorts, but not an AVOCADO_VERSION.

Expanding here a bit, for non-instrumented tests, it is possible to set AVOCADO_VERSION, AVOCADO_TEST_BASEDIR, AVOCADO_TEST_LOGDIR, and AVOCADO_TEST_OUTPUTDIR using the values from runnable config.

The problematic data we have right now are AVOCADO_TEST_WORKDIR and AVOCADO_TEST_LOGFILE. I don't think they are created for any kind of test that is not an instrumented test today.

Is it okay if we let those two out of nrunner for now?

Q) Should we deprecate the use of those environment variables for non-instrumented tests on nrunner?
A) I don't think we have an alternative for exec-test and related... deprecating or eliminating that completely would cause an unfavorable unbalance in the "reuse existing tests" side of the scale.

[richtja]

Expanding here a bit, for non-instrumented tests, it is possible to set AVOCADO_VERSION, AVOCADO_TEST_BASEDIR, AVOCADO_TEST_LOGDIR, and AVOCADO_TEST_OUTPUTDIR using the values from runnable config.

The problematic data we have right now are AVOCADO_TEST_WORKDIR and AVOCADO_TEST_LOGFILE. I don't think they are created for any kind of test that is not an instrumented test today.

Actually the AVOCADO_TEST_LOGFILE and others are not really used in instrumented test on the nrunner today. It is created the temporary file, but that is lost after the testing and those loges are not saved in ~/avocado/job-results/latest/test-results/. So the truth is that we are not using these variables even in instrumented tests on nrunner today.

Is it okay if we let those two out of nrunner for now?

[beraldoleal]

@willianrampazzo @richtja: Are you guys talking about avocado jobs get-output-files subcommand?

[willianrampazzo]

@willianrampazzo @richtja: Are you guys talking about avocado jobs get-output-files subcommand?

I think the output files are covered by AVOCADO_TEST_OUTPUTDIR, aren't they?

[richtja]

@willianrampazzo @richtja: Are you guys talking about avocado jobs get-output-files subcommand?

I think the output files are covered by AVOCADO_TEST_OUTPUTDIR, aren't they?

I think in the legacy runner it is true, but I don't know how is it in nrunner, because in the instrumented tests the AVOCADO_TEST_OUTPUTDIR is pointing to the temporal directory created by instrmented-runner.

[willianrampazzo]

Adding more context to the discussion, @beraldoleal gave the idea of reviewing the meaning of each variable. The AVOCADO_VERSION meaning is straightforward. After some investigation, I noticed that the environment variables are what their name says, related to the TEST, and not with those with a similar name on Avocado config.

Here is the output of avocado run --test-runner=nrunner examples/tests/env_variables.sh:

2021-08-25 17:16:44,050 process          L0461 DEBUG| [stdout] Avocado Version: 90.0
2021-08-25 17:16:44,050 process          L0730 INFO | Command '/home/wrampazz/src/avocado/avocado.dev/examples/tests/env_variables.sh' finished with 0 after 0.003338702s
2021-08-25 17:16:44,051 process          L0461 DEBUG| [stdout] Avocado Test basedir: /home/wrampazz/src/avocado/avocado.dev/examples/tests
2021-08-25 17:16:44,051 process          L0461 DEBUG| [stdout] Avocado Test workdir: /home/wrampazz/avocado/job-results/job-2021-08-25T17.16-8441f7c/test-results/tmp_dir04rct1cz/1-examples_tests_env_variables.sh
2021-08-25 17:16:44,052 process          L0461 DEBUG| [stdout] Avocado Test logdir: /home/wrampazz/avocado/job-results/job-2021-08-25T17.16-8441f7c/test-results/1-examples_tests_env_variables.sh
2021-08-25 17:16:44,052 process          L0461 DEBUG| [stdout] Avocado Test logfile: /home/wrampazz/avocado/job-results/job-2021-08-25T17.16-8441f7c/test-results/1-examples_tests_env_variables.sh/debug.log
2021-08-25 17:16:44,052 process          L0461 DEBUG| [stdout] Avocado Test outputdir: /home/wrampazz/avocado/job-results/job-2021-08-25T17.16-8441f7c/test-results/1-examples_tests_env_variables.sh/data

As you can see, those are related to the test execution. Basically, everything except for the Avocado Test Basedir has the logs directory from the config as reference. Here is my output from avocado config --datadir:

Avocado Data Directories:
   base    /home/wrampazz/avocado
   tests   /home/wrampazz/src/avocado/avocado.dev/examples/tests
   data    /home/wrampazz/avocado/data
   logs    /home/wrampazz/avocado/job-results
   cache   /home/wrampazz/avocado/data/cache

Considering the nrunner allows spawners anywhere different from the host system, the only idea I had was to check if the directories exist; if not, create them, but is there any value for the user here? The user will need to access the target location to look at the content.

Comments, again, are appreciated @beraldoleal, @clebergnu, @richtja, @ana.

[clebergnu]

Before I comment, let me just record that while working on the switch to the nrunner, specifically on optional_plugins/varianter_yaml_to_mux/tests/test_functional.py:MultiplexTests.test_run_mplex_params I came across this exact point.

We have a sample test, examples/tests/env_variables.sh, that is used on that functional test. Example of behavior with the current runner:

$ avocado run --test-runner=runner examples/tests/env_variables.sh
JOB ID     : bae1cd87b4a48a7be95202a19c05797a8635d2bb
JOB LOG    : /home/cleber/avocado/job-results/job-2021-08-26T09.18-bae1cd8/job.log
 (1/1) examples/tests/env_variables.sh: PASS (0.01 s)
RESULTS    : PASS 1 | ERROR 0 | FAIL 0 | SKIP 0 | WARN 0 | INTERRUPT 0 | CANCEL 0
JOB HTML   : /home/cleber/avocado/job-results/job-2021-08-26T09.18-bae1cd8/results.html
JOB TIME   : 1.79 s

Comparing with nrunner:

$ avocado run --test-runner=nrunner examples/tests/env_variables.sh
JOB ID     : e5b9211f88c012f04ca4f55320e03f7fbf0e1abe
JOB LOG    : /home/cleber/avocado/job-results/job-2021-08-26T09.18-e5b9211/job.log
 (1/1) examples/tests/env_variables.sh: STARTED
 (1/1) examples/tests/env_variables.sh: FAIL (0.01 s)
RESULTS    : PASS 0 | ERROR 0 | FAIL 1 | SKIP 0 | WARN 0 | INTERRUPT 0 | CANCEL 0
JOB HTML   : /home/cleber/avocado/job-results/job-2021-08-26T09.18-e5b9211/results.html
JOB TIME   : 1.11 s

[willianrampazzo]

Yep, I saw that, and I was expecting to change this for the switch if we decide to reduce the supported number of environment variables.