Multiple TestSuites: Decide approach with multiple configs settings

[beraldoleal]

We are moving forward trying to add support to multiple Test Suites in a single Job. This will bring power to Avocado but also brings to us some challenges.

The current avocado Job implementation always had in mind only one test suite and it is constantly querying some configuration options and taking some actions. IMO, we need to review each of the configuration options that we are using today inside Job() class and decide what to do with this option: i.e: it will be moved to TestSuite() class? Will be kept inside the Job() class? Do we really need this inside the Job() class?

Before I list here all the settings, let-me just tell why I'm concerned about this: How we are going to maintan consistency among all configuration dicts (when using multiple test suites) ? For instance: lets say that the user creates two test suites with two config dicts:

suite1 = TestSuite.from_config( {'run.references: ['/bin/true']', 'run.keep_tmp': True})
suite2 = TestSuite.from_config( {'run.references: ['/bin/true']', 'run.keep_tmp': False})
with Job([suite1, suite2]) as job:
  job.run()

Here, I used run.keep_tmp just to give one example and because this option is being used inside the Job() class, but could be any of the settings used inside the Job() class. So, what option Job will use in fact?

To give more examples, here is the list of all options that are being used inside the Job() class:

1. job.output.loglevel
2. job.run.timeout
3. run.dry_run.enabled
4. run.unique_job_id
5. run.job_category
6. run.test_parameters
7. run.dry_run.no_cleanup
8. run.keep_tmp
9. run.store_logging_stream
10. sysinfo.collect.enabled
11. replay_sourcejob
12. core.show

Some options are obvious: for instance: run.test_parameters unless I'm missing something, this option needs to be moved to the TestSuite() class, because the user could need run two test suites with different test_parameters. What about the others?

replay_sourcejob is an example case where we didn't migrate to the new model yet.

Everything inside job.* makes sense to keep inside the job class. But how we are going to guarantee consistency? Let's say that config dicts have different job.output.loglevel? What is going to be used by the Job()?

Comments here? @willianrampazzo @clebergnu ?

[beraldoleal]

This is related to #3958

[willianrampazzo]

We are moving forward trying to add support to multiple Test Suites in a single Job. This will bring power to Avocado but also brings to us some challenges.

"With great power comes great responsibility" - Ben, Uncle.

The current avocado Job implementation always had in mind only one test suite and it is constantly querying some configuration options and taking some actions. IMO, we need to review each of the configuration options that we are using today inside Job() class and decide what to do with this option: i.e: it will be moved to TestSuite() class? Will be kept inside the Job() class? Do we really need this inside the Job() class?

It seems to make sense to me.

Before I list here all the settings, let-me just tell why I'm concerned about this: How we are going to maintan consistency among all configuration dicts (when using multiple test suites) ? For instance: lets say that the user creates two test suites with two config dicts:

suite1 = TestSuite.from_config( {'run.references: ['/bin/true']', 'run.keep_tmp': True})
suite2 = TestSuite.from_config( {'run.references: ['/bin/true']', 'run.keep_tmp': False})
with Job([suite1, suite2]) as job:
  job.run()

Here, I used run.keep_tmp just to give one example and because this option is being used inside the Job() class, but could be any of the settings used inside the Job() class. So, what option Job will use in fact?

Okay, I see the issue now.

To give more examples, here is the list of all options that are being used inside the Job() class:

1. job.output.loglevel
2. job.run.timeout
3. run.dry_run.enabled
4. run.unique_job_id
5. run.job_category
6. run.test_parameters
7. run.dry_run.no_cleanup
8. run.keep_tmp
9. run.store_logging_stream
10. sysinfo.collect.enabled
11. replay_sourcejob
12. core.show

Some options are obvious: for instance: run.test_parameters unless I'm missing something, this option needs to be moved to the TestSuite() class, because the user could need run two test suites with different test_parameters. What about the others?

Reading until here, I may say to move everything that is not Job-specific to the test suite. Let me continue reading.

replay_sourcejob is an example case where we didn't migrate to the new model yet.

Everything inside job.* makes sense to keep inside the job class. But how we are going to guarantee consistency? Let's say that config dicts have different job.output.loglevel? What is going to be used by the Job()?

Comments here? @willianrampazzo @clebergnu ?

Okay, I agree with you that we need to look at one-by-one of the options and decide where it should take effect. It is not a trivial task and will extend the current test execution behavior, depending on what we choose.

For example, job.output.loglevel for me seems to be part of the Job. All test suites must follow what is set there. But as you mentioned, how to enforce which option takes precedence if it is defined into more than one test suite. As an idea, would it be possible to set one config for the Job and one config for the test suite? Job-related options would be defined in the Job config, should be unique, and would affect the Job, while the test suite related config would go to the test suite level, but could still have access to the unique Job config. I don't know if it is feasible; it is just one idea that came to my mind.

How ugly is it:

job_config = {'job.output.loglevel': '3'}
suite1 = TestSuite.from_config( {'run.references: ['/bin/true']', 'run.keep_tmp': True})
suite2 = TestSuite.from_config( {'run.references: ['/bin/true']', 'run.keep_tmp': False})
with Job(job_config, [suite1, suite2]) as job:
    job.run()

Or a less ugly option:

suite1 = TestSuite.from_config( {'run.references: ['/bin/true']', 'run.keep_tmp': True})
suite2 = TestSuite.from_config( {'run.references: ['/bin/true']', 'run.keep_tmp': False})
job_config = {'job.output.loglevel': '3', 'job.suites': [suite1, suite2]}

with Job(job_config) as job:
    job.run()

I think this makes the handling of the test suites less painful and well-separated into the Job class.

Extending the discussion here, if we decouple the test suite from the Job and create a communication interface between then (and possibly all Avocado modules), I think it would benefit the scheduler we are discussing on BP003.

[beraldoleal]

For example, job.output.loglevel for me seems to be part of the Job. All test suites must follow what is set there. But as you mentioned, how to enforce which option takes precedence if it is defined into more than one test suite. As an idea, would it be possible to set one config for the Job and one config for the test suite? Job-related options would be defined in the Job config, should be unique, and would affect the Job, while the test suite related config would go to the test suite level, but could still have access to the unique Job config. I don't know if it is feasible; it is just one idea that came to my mind.

How ugly is it:

job_config = {'job.output.loglevel': '3'}
suite1 = TestSuite.from_config( {'run.references: ['/bin/true']', 'run.keep_tmp': True})
suite2 = TestSuite.from_config( {'run.references: ['/bin/true']', 'run.keep_tmp': False})
with Job(job_config, [suite1, suite2]) as job:
    job.run()

For me, this is the way to go, except for the first argument (job_config). Having this configuration separation it is painful for the user.

This is the "consistency problem that I see here". We are trying to use a "unified dict". But this is "inconsistent" with the idea of multiple test suites. IMO. If we don't solve this now, we can have some ugly "gambiarras" (workarounds) inside the code.

Or a less ugly option:

suite1 = TestSuite.from_config( {'run.references: ['/bin/true']', 'run.keep_tmp': True})
suite2 = TestSuite.from_config( {'run.references: ['/bin/true']', 'run.keep_tmp': False})
job_config = {'job.output.loglevel': '3', 'job.suites': [suite1, suite2]}

with Job(job_config) as job:
    job.run()

In theory a dict config should be plain so we can serialize and unserialize. Having TestSuite() objects inside the config sounds strange to me.

But I think that besides the way to represent here, we need to decide the precedence. In your second example, you are assuming that run.keep_tmp is not used anymore inside the Job(), right? So again, we need to decide one by one, all options listed here.

I think this makes the handling of the test suites less painful and well-separated into the Job class.

Extending the discussion here, if we decouple the test suite from the Job and create a communication interface between then (and possibly all Avocado modules), I think it would benefit the scheduler we are discussing on BP003.

I agree with this. But IMO, it is a different problem. I can't how this will help here.

[beraldoleal]

A possible alternative it is having a validation where everything inside job.* namespace should be equal in all TestSuite's configs. In that way, inside the Job() class we could copy just the first test_suite.config everything that starts with job.* and we will be with a clear conscience that this is consistent.

But again, I'm assuming that everything inside the Job() class is only using settings inside the job.*.

[richtja]

A possible alternative it is having a validation where everything inside job. namespace should be equal in all TestSuite's configs. In that way, inside the Job() class we could copy just the first test_suite.config everything that starts with job. and we will be with a clear conscience that this is consistent.

For me, this is a better solution than have two different config files. But I think that for a user it can be confusing because the configuration of Job() will be inside the first test_suite.config.

What about combined these two solutions together and have test_suite.config inside job.config?

job_config = {'job.output.loglevel': '3', 'job.suites': [{'run.references: ['/bin/true']', 'run.keep_tmp': True}, {'run.references: ['/bin/true']', 'run.keep_tmp': False}]}

with Job(job_config) as job:
    job.run()

[beraldoleal]

@richtja I like partially your idea, but I still have some questions:

1. What is part of the job config? Everything that is starting with job.* namespace? And the other options? Are we assuming that everything that don't start with job.* is about the suite config? This brings us to the point that we might need to review all the options one by one.

2. We need to pass the TestSuite() to the Job(), and TestSuite needs the configuration to be created. How we are going to pass that?

[richtja]

What is part of the job config? Everything that is starting with job. namespace? And the other options? Are we assuming that everything that don't start with job. is about the suite config? This brings us to the point that we might need to review all the options one by one.

Yes, I agree with you that we have to review all the options one by one.

We need to pass the TestSuite() to the Job(), and TestSuite needs the configuration to be created. How we are going to pass that?

Actually I don't have a nice solution for that, I have an idea that the TestSuite() can be created inside Job() based on the configuration, but I think that this brings more problems than it solves.

[clebergnu]

Let's assume that we want to preserve the current and simplest use case possible:

import sys
from avocado import Job

with Job() as job:
   sys.exit(job.run())

References are the most common way of populating a Job test suite, but there are other ways. For instance Avocado-VT can load tests from a configuration file. So, the following example code would achieve that:

import sys
from avocado import Job

with Job({'vt.config': '~/.config/avocado-vt.config'}) as job:
   sys.exit(job.run())

And if Avocado-VT developers would choose to add default ="~/.config/avocado-vt.config" to the vt.config option registration, they should be able to (why shouldn't they?). Then, with Avocado-VT installed, the first code snippet would behave like the second. A pip install avocado-fictional-plugin-load-random-tests-from-test-dir is another (this one is clearly a fictional example).

If it's not clear by now, we should not require that test suites are created from Job arguments. Not respecting that use case will, among other things, break Avocado-VT.

The novel use case is allowing multiple test suites, and doing so in a way that is as simple as possible implementation wise, and has good usability to users. My first hard opinion is that we don't have to require the newbie user to interact with anything but the Job class and configuration as data. What I mean is:

import sys
from avocado import Job

CONFIG_DATA = {'run.test_runner': 'nrunner'}

with Job(CONFIG_DATA) as job:
   sys.exit(job.run())

I don't see why the same could not be expanded to the multiple test suite approach, like @richtja suggested. That is:

job_config = {'job.output.loglevel': '3',
              'job.suites': [{'run.references: ['/bin/true']', 'run.keep_tmp': True},
                             {'run.references: ['/bin/true']', 'run.keep_tmp': False}]}

with Job(job_config) as job:
    job.run()

On your reply ( https://github.com/avocado-framework/avocado/issues/4019#issuecomment-663547372 ) you mention that "We need to pass the TestSuite() to the Job(), and TestSuite needs the configuration to be created. How we are going to pass that?" but I fail to see why we need to pass the TestSuite() to the Job(). You're creating a requirement that is artificially blocking that proposal/option. In my understanding, the TestSuite() can still be created by the job, cased on descriptive data. Do you see a blocker or a reason for not allowing that (and thus *requiring the user to interact with a TestSuite()) ?

Finally, the last problem has to with the scope of configuration, that is if features can be applied at the test suite level. I don't think going over every single Job API option, and checking if they behave/apply perfectly within the scope of a TestSuite, as opposed to the Job as a whole, is something we can achieve right now.

And that's OK :)

For instance, changing the verbosity level (job.output.loglevel) between one test suite and another may not be effective (because of $TOO_MANY_IMPLEMENTATION_REASONS, such as the human UI plugin being loaded very early and having no hook for changing behavior between suites). I believe we can still attempt to name the options as well as possible, but we should not attempt to prove that for every option, "A" is effective when applied to a test suite, and option B is not. If we get to test an option and validate that it's effective at the test suite level, than we document that aspect. This is going to solve the majority of use cases we have, such as passing different variants configuration to different test suites.

And to not lose sanity in the process, I think we need to have a very clear implementation, such as that:

1. Every code that gets called from a test suite execution context passes its own configuration forward
2. The previous point assumes that every test suite has a complete configuration copy
3. The previous point suggests that we won't require users to create these complete configurations, but rather merge the suite specific configuration with the global job configuration

That way, if we do:

job_config = {'job.output.loglevel': '3',
              'job.suites': [{'run.references: ['/bin/true']', 'run.keep_tmp': True},
                             {'run.references: ['/bin/true']', 'run.keep_tmp': False}]}

with Job(job_config) as job:
    job.run()

The Job implementation will create:

self.test_suites = [TestSuite({'job.output.loglevel': '3', 'run.references: ['/bin/true']', 'run.keep_tmp': True}),
                    TestSuite({'job.output.loglevel': '3', 'run.references: ['/bin/true']', 'run.keep_tmp': False})]

And the code that will execute the tests from a test suite will do something like:

def run_test_suite(test_suite):
   if test_suite.config.get('run.keep_tmp'):
       # do something

Let me know if I'm missing something here, and thanks for the discussion and work here.

[beraldoleal]

@clebergnu I'm ok with not passing the TestSuites to the Job() for now.

My only problem is the configuration consistency. Our current model of "namespace" already provides this 'separation' (the separation is the namespace itself, the dots). So, for instance, everything that starts with job. should be related to the job. I'm ok with creating a specific key (job.suites) to pass the specific configs. I'm having problems to accept the inconsistency.

From the user perspective if I create a suite = TestSuite.from_config(config) and on that suite, I passed run.dry_run.enabled: True, this should be respected, the problem is that this dry_run is used today on the Job() side.

I'm trying to make things consistent and avoid confusion here. What is going to happen with this?

job_config = {'run.dry_run.enabled': False,
              'job.suites': [{'run.references: ['/bin/true']', 'run.dry_run.enabled': True},
                             {'run.references: ['/bin/true']', 'run.dry_run.enabled': False}]}

Allowing this and using only the first one (False), IMO it is inconsistent and makes things confusing.

1. Are we moving all other options that don't start with job.* to job.suites?
2. Are we ignoring everything that starts with job.* inside the TestSuite run?

IMO, stick with one plain dict for each test suite could be better.

When I said to review the options, are not all options, but the only ones that I listed here on my first comment.

Also, with this requirement of having a job.suites key, will also break the current code (including Avocado-vt) right? I was trying to think about a solution that would not require such separation on the user's configuration. (maybe the break/update is inevitable with this update)

[beraldoleal]

Yet on the "consistency": so far, every option could be set via config file by a section + key (namespace). We know that configparser is very limited to represent lists. So how do you imagine this job.suites nested config inside a config file? And why all other sections inside this file are represented as a flat dict and this one is not?

[clebergnu]

@clebergnu I'm ok with not passing the TestSuites to the Job() for now.

My only problem is the configuration consistency. Our current model of "namespace" already provides this 'separation' (the separation is the namespace itself, the dots). So, for instance, everything that starts with job. should be related to the job. I'm ok with creating a specific key (job.suites) to pass the specific configs. I'm having problems to accept the inconsistency.

I'm not sure I understood your proposal, is it something like the following?

{'json.suites.first_suite.references': ['/bin/true'],
 'json.suites.first_suite.run.dry_run.enabled': True}

From the user perspective if I create a suite = TestSuite.from_config(config) and on that suite, I passed run.dry_run.enabled: True, this should be respected, the problem is that this dry_run is used today on the Job() side.

That's one of the reasons I would avoid attempting a strict separation from what is test suite's and what is something else... I don't think there's a dichotomy here, and attempting to make things fit this way will cause frustration.

I'm trying to make things consistent and avoid confusion here. What is going to happen with this?

job_config = {'run.dry_run.enabled': False,
              'job.suites': [{'run.references: ['/bin/true']', 'run.dry_run.enabled': True},
                             {'run.references: ['/bin/true']', 'run.dry_run.enabled': False}]}

It's very easy to be consistent in those situations, and I mean consistent with the behavior we document. If we say that the suite configuration takes precedence (which I believe is the whole point of this feature), then the result is:

job_config = {'run.dry_run.enabled': False,
               'job.suites': [{'run.references: ['/bin/true']', 'run.dry_run.enabled': True},
                              {'run.references: ['/bin/true']', 'run.dry_run.enabled': False}]}
with Job(job_config) as job:
   assert job.suites[0].config.get('run.dry_run.enabled') == True
   assert job.suites[1].config.get('run.dry_run.enabled') == False

And the implementation would look something like:

class Job():
   def __init__(config):
       self.config = config
       self.test_suites = []

   def create_test_suites(self):
      for suite_config in self.config.get('job.suites'):
         config = copy.copy(self.config)
         config.update(suite_config)
         self.test_suites.append(TestSuite(config))

Allowing this and using only the first one (False), IMO it is inconsistent and makes things confusing.

I don't see how that is inconsistent if we document the precedence order. It's exactly how we do regarding the precedence order of builtin defaults, configuration files, and command line arguments. They're defined at multiple levels, and one of them, if available, prevails over the others.

1. Are we moving all other options that don't start with job.* to job.suites?
2. Are we ignoring everything that starts with job.* inside the TestSuite run?

I believe we need to focus on behavior and choose good names. For instance, run.job.category has a legacy name, because it comes (I believe) from the run CLICmd Plugin. But that option is really about setting the category for the job as whole, and I wouldn't mind renaming it to simply job.category. If someone passes that to a test suite, it will have no effect in the context of the test suite, but will cause no harm.

There may be cases where a simpler name, say timeout will be confusing indeed, as it may apply to either the job or the suite. But, remember that we have no concept of multiple test suites, so the existing options, say job.run.timeout will only apply to the job as a whole (there will be no code to attempt to get that value and timeout a test suite). When that code is written, then we can distinguish between job.timeout and suite.timeout.

IMO, stick with one plain dict for each test suite could be better.

I don't think I understand because one dict is being given to each test suite in the previous example... Is the list assignment what you're describing as a negative thing? I tend to agree that is non-obvious indeed, but then the other option would be to require a name for every test suite?

When I said to review the options, are not all options, but the only ones that I listed here on my first comment.

OK, I see. I hope the timeout example gives a general idea of how I believe we should treat this.

Also, with this requirement of having a job.suites key, will also break the current code (including Avocado-vt) right? I was trying to think about a solution that would not require such separation on the user's configuration. (maybe the break/update is inevitable with this update)

I don't think so, as there's already a test_suite attribute, and a suites will be an extension. But we need to double check indeed, and we can adjust Avocado-VT if needed.

[beraldoleal]

@clebergnu I'm ok with not passing the TestSuites to the Job() for now. My only problem is the configuration consistency. Our current model of "namespace" already provides this 'separation' (the separation is the namespace itself, the dots). So, for instance, everything that starts with job. should be related to the job. I'm ok with creating a specific key (job.suites) to pass the specific configs. I'm having problems to accept the inconsistency.

I'm not sure I understood your proposal, is it something like the following?

{'json.suites.first_suite.references': ['/bin/true'],
 'json.suites.first_suite.run.dry_run.enabled': True}

For now, I'm just trying to collect ideas and double-check if they are consistent. We could remove the prefix json.suites here if we stick with this solution.

But I'm trying to imagine how @richtja proposal would be represented inside a config file. If we take that: 1) we do not accept nested sections inside config files (configparser limitation) and 2) configparser support for lists is very limited, how we would represent this inside the config file?

Yes, I don't like much this representation here, and I'm still trying to find a better solution. But so far is the "most consistent representation" that we have.

I'm trying to make things consistent and avoid confusion here. What is going to happen with this?

job_config = {'run.dry_run.enabled': False,
              'job.suites': [{'run.references: ['/bin/true']', 'run.dry_run.enabled': True},
                             {'run.references: ['/bin/true']', 'run.dry_run.enabled': False}]}

It's very easy to be consistent in those situations, and I mean consistent with the behavior we document. If we say that the suite configuration takes precedence (which I believe is the whole point of this feature), then the result is:

job_config = {'run.dry_run.enabled': False,
               'job.suites': [{'run.references: ['/bin/true']', 'run.dry_run.enabled': True},
                              {'run.references: ['/bin/true']', 'run.dry_run.enabled': False}]}
with Job(job_config) as job:
   assert job.suites[0].config.get('run.dry_run.enabled') == True
   assert job.suites[1].config.get('run.dry_run.enabled') == False

As I mentioned, I would like much to avoid a documentation reading for our "configuration nested syntax only on specific cases". This IMO would increase the learning curve. For such a trivial task, as configuration should be, this could be more intuitive. But ok, let's say that we will document this, I do have one question:

How we are going to represent this inside the config file? IMO having a 1:1 mapping of dict and config file was a premise so far.

1. Are we moving all other options that don't start with job.* to job.suites?
2. Are we ignoring everything that starts with job.* inside the TestSuite run?

I believe we need to focus on behavior and choose good names. For instance, run.job.category has a legacy name, because it comes (I believe) from the run CLICmd Plugin. But that option is really about setting the category for the job as whole, and I wouldn't mind renaming it to simply job.category. If someone passes that to a test suite, it will have no effect in the context of the test suite, but will cause no harm.

Yes! Oh Yes!! That is exactly one of the goals here: review each of 12 items (found inside Job() class) so this will be much less confusing. One down, thanks. (And all others that starts with job. already). I'm ok with ignoring job. inside the TestSuite because the "enforcement" will be the namespace. This is the consistency that I'm looking for.

There may be cases where a simpler name, say timeout will be confusing indeed, as it may apply to either the job or the suite. But, remember that we have no concept of multiple test suites, so the existing options, say job.run.timeout will only apply to the job as a whole (there will be no code to attempt to get that value and timeout a test suite). When that code is written, then we can distinguish between job.timeout and suite.timeout.

Agreed and I'm more than ok with that.

IMO, stick with one plain dict for each test suite could be better.

I don't think I understand because one dict is being given to each test suite in the previous example... Is the list assignment what you're describing as a negative thing? I tend to agree that is non-obvious indeed, but then the other option would be to require a name for every test suite?

See my previous comment, I'm trying to see this inside the config file. I don't like having to name the test suites too, but in favor of the consistency, I think that I can survive with that.

When I said to review the options, are not all options, but the only ones that I listed here on my first comment.

OK, I see. I hope the timeout example gives a general idea of how I believe we should treat this.

Yes, and was what I was expecting. But we still have a few other options that might change the behavior with this feature. For instance: replay_job. Are we going to allow a replay of a specific test suite? Or only replays of the entire job? If the last, should we rename this option to job.something?

[clebergnu]

@richtja I like partially your idea, but I still have some questions:

1. What is part of the job config? Everything that is starting with job.* namespace? And the other options? Are we assuming that everything that don't start with job.* is about the suite config? This brings us to the point that we might need to review all the options one by one.

2. We need to pass the TestSuite() to the Job(), and TestSuite needs the configuration to be created. How we are going to pass that?

@clebergnu I'm ok with not passing the TestSuites to the Job() for now. My only problem is the configuration consistency. Our current model of "namespace" already provides this 'separation' (the separation is the namespace itself, the dots). So, for instance, everything that starts with job. should be related to the job. I'm ok with creating a specific key (job.suites) to pass the specific configs. I'm having problems to accept the inconsistency.

I'm not sure I understood your proposal, is it something like the following?

{'json.suites.first_suite.references': ['/bin/true'],
 'json.suites.first_suite.run.dry_run.enabled': True}

For now, I'm just trying to collect ideas and double-check if they are consistent. We could remove the prefix json.suites here if we stick with this solution.

OK. I meant job.suites BTW :/

But I'm trying to imagine how @richtja proposal would be represented inside a config file. If we take that: 1) we do not accept nested sections inside config files (configparser limitation) and 2) configparser support for lists is very limited, how we would represent this inside the config file?

That's actually one of the points of the Job API... we can have more expressiveness than with configuration files and command line arguments. So I think this is actually consistent with the approach of scaling to more capable means when you have more complex problems. In short, there would be no backwards mapping (from the configuration dictionary to the configuration file or command line arguments).

Yes, I don't like much this representation here, and I'm still trying to find a better solution. But so far is the "most consistent representation" that we have.

I'm trying to make things consistent and avoid confusion here. What is going to happen with this?

job_config = {'run.dry_run.enabled': False,
              'job.suites': [{'run.references: ['/bin/true']', 'run.dry_run.enabled': True},
                             {'run.references: ['/bin/true']', 'run.dry_run.enabled': False}]}

It's very easy to be consistent in those situations, and I mean consistent with the behavior we document. If we say that the suite configuration takes precedence (which I believe is the whole point of this feature), then the result is:

job_config = {'run.dry_run.enabled': False,
               'job.suites': [{'run.references: ['/bin/true']', 'run.dry_run.enabled': True},
                              {'run.references: ['/bin/true']', 'run.dry_run.enabled': False}]}
with Job(job_config) as job:
   assert job.suites[0].config.get('run.dry_run.enabled') == True
   assert job.suites[1].config.get('run.dry_run.enabled') == False

As I mentioned, I would like much to avoid a documentation reading for our "configuration nested syntax only on specific cases". This IMO would increase the learning curve. For such a trivial task, as configuration should be, this could be more intuitive. But ok, let's say that we will document this, I do have one question:

How we are going to represent this inside the config file? IMO having a 1:1 mapping of dict and config file was a premise so far.

No, it's definitely not. And that's why we have shifted from presenting the (ini-like) configuration file on the job to the configuration dictionary. And that's why we want jobs to be replayable with the dictionary, and never with config files. Just like command line arguments, the configuration files can contribute to creating the dictionary, but the reversal of that is not a requirement.

1. Are we moving all other options that don't start with job.* to job.suites?
2. Are we ignoring everything that starts with job.* inside the TestSuite run?

I believe we need to focus on behavior and choose good names. For instance, run.job.category has a legacy name, because it comes (I believe) from the run CLICmd Plugin. But that option is really about setting the category for the job as whole, and I wouldn't mind renaming it to simply job.category. If someone passes that to a test suite, it will have no effect in the context of the test suite, but will cause no harm.

Yes! Oh Yes!! That is exactly one of the goals here: review each of 12 items (found inside Job() class) so this will be much less confusing. One down, thanks. (And all others that starts with job. already). I'm ok with ignoring job. inside the TestSuite because the "enforcement" will be the namespace. This is the consistency that I'm looking for.

OK, I get your point. I guess we're talking about the same thing. I believe the review can come now and/or during the implementation of the feature tests for those.

There may be cases where a simpler name, say timeout will be confusing indeed, as it may apply to either the job or the suite. But, remember that we have no concept of multiple test suites, so the existing options, say job.run.timeout will only apply to the job as a whole (there will be no code to attempt to get that value and timeout a test suite). When that code is written, then we can distinguish between job.timeout and suite.timeout.

Agreed and I'm more than ok with that.

IMO, stick with one plain dict for each test suite could be better.

I don't think I understand because one dict is being given to each test suite in the previous example... Is the list assignment what you're describing as a negative thing? I tend to agree that is non-obvious indeed, but then the other option would be to require a name for every test suite?

See my previous comment, I'm trying to see this inside the config file. I don't like having to name the test suites too, but in favor of the consistency, I think that I can survive with that.

Forget about the config file, like I said before, reversing from the dictionary to the configuration file was never a requirement.

When I said to review the options, are not all options, but the only ones that I listed here on my first comment.

OK, I see. I hope the timeout example gives a general idea of how I believe we should treat this.

Yes, and was what I was expecting. But we still have a few other options that might change the behavior with this feature. For instance: replay_job. Are we going to allow a replay of a specific test suite? Or only replays of the entire job? If the last, should we rename this option to job.something?

There is no separate test suite implemented so far, so I expect that we keep the features we have handling the entire job for now. Let's not get concerned with attempting to add features to work with test suites. The most important ones, like parameters/variants will most probably work transparently because they are either very close to the test execution or are being moved there. So, for replay_job, yes, a rename is a good idea: job.replay ?

[pevogam]

Hi all, looking at some of the PRs related to this and the discussion here and somewhat from an external point of view I was wondering why is all of this extra complexity needed? Can you mention a use case or two about multiple test suites (and I mean "test suite" as implementation agnostic term the way a general user would understand it)? In particular, what are the advantages this feature offers in comparison to simply running multiple jobs?

[pevogam]

I just saw the original issue proposing this #3958, will post my comments there. At least from viewing this issue it seems to be large an unnecessary complication of settings API that just started to stabilize.

[clebergnu]

Hi all, looking at some of the PRs related to this and the discussion here and somewhat from an external point of view I was wondering why is all of this extra complexity needed? Can you mention a use case or two about multiple test suites (and I mean "test suite" as implementation agnostic term the way a general user would understand it)? In particular, what are the advantages this feature offers in comparison to simply running multiple jobs?

Hi @pevogam, your point is very valid: we're letting go of some straightforwardness here so some explanation is needed.

Based on our own, and some use cases we learned from the community, people are using multiple avocado run ... executions to establish that their tests passed (or fail). While there may be situations where users want to run multiple avocado jobs, most of the examples we found point to one, or many, Avocado limitations.

The most striking current limitation is being able to map different test parameters to different tests. Usually, those tests parameters come from varianter implementations (most often the "Yaml To Mux" files). So getting down to use cases, it's very common to find use cases such as:

• test_cpus.py
• test_network_devices.py

Where I want to run the test_cpus.py with parameters model=Nehalem and model=SandyBridge, and test_network_devices.py with parameters model=virtio and model=e1000. What we realized is that this is just one example, and instead of creating a custom parameter or varianter "bin" for each test, it made sense to go beyond and allow for the grouping of tests in a job, and allow for those groups to receive custom configurations just like the Job API allows for the job as a whole now.

Other use cases can be found on Avocado itself, for instance:

• https://github.com/avocado-framework/avocado/blob/33287f055bb40233929b755a43e82a8aa2fc9bf6/selftests/pre_release/jobs/timesensitive.py
• https://github.com/avocado-framework/avocado/blob/33287f055bb40233929b755a43e82a8aa2fc9bf6/selftests/pre_release/jobs/vmimage.py

Shows two jobs which should really be one, that is, Avocado's pre-release job. We need two jobs at the moment, because we can not configure distinguishing aspects for different tests, which this work aims to allow.

Please let m know if this makes sense to you.

[pevogam]

I see, now that I read this explanation I can confirm that on our side we have experienced this limitation plenty of times too, specifically on the command line. We still use the Cartesian config provided from legacy Autotest though and at least within the config files we could easily specify different configurations per test variants. I have glanced through the YAML2MUX varianter in the past in order to compare it with the Cartesian configuration/varianter and I think the same should be possible to achieve there.

So for your example with test_cpus.py and test_network_devices.py do you mean this as a restriction on the command line specifically or perhaps in some greater sense?

[clebergnu]

I see, now that I read this explanation I can confirm that on our side we have experienced this limitation plenty of times too, specifically on the command line. We still use the Cartesian config provided from legacy Autotest though and at least within the config files we could easily specify different configurations per test variants. I have glanced through the YAML2MUX varianter in the past in order to compare it with the Cartesian configuration/varianter and I think the same should be possible to achieve there.

So for your example with test_cpus.py and test_network_devices.py do you mean this as a restriction on the command line specifically or perhaps in some greater sense?

It's both in the command line an in a greater sense.

The Job API IMO solves most of the command line expressiveness limitations, but, the only way to use the cartesian config or the Yaml to Mux varianter accross different tests with different parameters, is if they all agree to the same structure.

This may be seen as a positive aspect for something like Avocado-VT, in which you do want to have a project-wide standard and the entry point into the test suite is the cartesian configuration itself, but it fails miserably on individual tests that have their individual parameters. For instance, hooking a completely new test into Avocado-VT may require a lot of only this and no that just to get along with the variants and structure in place.

Also, other variants such as the CIT perform at their best while creating variants with parameters that are really relevant to a test. Adding the ~200 key/values as input to CIT where a test will only use a few is a huge overkill.

Finally, the implementation for test runners that we have right now have the responsibility of running one test suite. It means that, with the Job API, we can one job configured to run one suite on the local system, while another somewhere else, and still present a unified result. I think the possibilities are endless and the complexity added here is small when compared to the use cases that we can solve straight away.

Hope it makes sense! Thanks!

[pevogam]

The Job API IMO solves most of the command line expressiveness limitations, but, the only way to use the cartesian config or the Yaml to Mux varianter accross different tests with different parameters, is if they all agree to the same structure.

I guess what limits my understanding here is the fact that I rely on the same type of tests for all of my needs and often haven't really understood the need to use other test types. No matter what language a test is typed in, as long as it gets a config dictionary that could be tailored and scoped specifically to it either via config file or command line, I think everybody is happy. Correct me if I miss something of great importance but even the current discussion is not that much about some exotic differences in test structure and requirements as much as it is about this need to provide tailed configuration within some scope. The latter than sounds more like the job of a varianter than of the entire avocado command line.

This may be seen as a positive aspect for something like Avocado-VT, in which you do want to have a project-wide standard and the entry point into the test suite is the cartesian configuration itself, but it fails miserably on individual tests that have their individual parameters.

I don't understand this part quite well. The point of the Cartesian and any other varianter is precisely this - to provide individual parameters to tests. I guess you also mean that varianters also decide what final tests will be parsed in addition to providing them with unique configuration parameters which is a fair point.

For instance, hooking a completely new test into Avocado-VT may require a lot of only this and no that just to get along with the variants and structure in place.

Not if the variant names are unique enough and with some thousand tests by now I have hardly had to use more then 1-2 restrictions like this.

Also, other variants such as the CIT perform at their best while creating variants with parameters that are really relevant to a test. Adding the ~200 key/values as input to CIT where a test will only use a few is a huge overkill.

The way the variants are defined in a Cartesian or YAML2MUX varianter can go anywhere from few to many parameters and this strictly depends on the use cases so I am not sure where the 200 key/value pairs come from. I guess such parameter redundancies could be attributed to limitations in some of the varianters. If you mean this in particular as something which is solved via shared parameter scopes across tests (here coined test suites) then for me these remains just test variant scopes and thus subject of the choice of varianter again.

Finally, the implementation for test runners that we have right now have the responsibility of running one test suite. It means that, with the Job API, we can one job configured to run one suite on the local system, while another somewhere else, and still present a unified result. I think the possibilities are endless and the complexity added here is small when compared to the use cases that we can solve straight away.

So to rephrase this advantage (to see if I understand it correctly), you mean that with a multi-suite job we can support test running also with multiple runners (and thus spawners/containers/platforms)?

Variant scopes could take care of different platforms of execution, test types, etc. so long as there is a unified way to pass the output of a varianter (e.g. list of dictionaries with parameters for each test in JSON or some other standard format) to a runner or a run managing component.

I think most of this boils down to correctly separating test configuration originating from avocado config files and command line with test configuration originating from test varianters. The desire to provide parameters of various sharing scopes to tests that could go down to the individual test seems really familiar implementing half of a varianter into the avocado settings.

Adapting an avocado plugin to the new settings made me wonder about this too: how much are we differentiating between configuration set in config files of special format (e.g. YAML or Cartesian) and configuration set in the default (empty or not) avocado configs? There is a similar confusion in Avocado VT as well where one would define e.g. default Qemu binary in both base.cfg and the vt.conf. Is there a clear line where one can say "this configures a test" and "this configures the plugins"? Because it seems many of us are confused about the current presence of dual configuration.

[willianrampazzo]

Hi @pevogam, as a real-world example, here how the multiple test suites can help with different combinations of parameters in the config: https://github.com/avocado-framework/avocado/pull/4079.

[pevogam]

Hi @pevogam, as a real-world example, here how the multiple test suites can help with different combinations of parameters in the config: #4079.

Hi @willianrampazzo, so this then confirms my suspicions above that what we are talking for is a varianter functionality to be added to the job API. I wonder if there is a more modular way of achieving this (considering we already have interchangeable varianter plugins) but I guess it is good enough for the time being.

However, I still wonder about yours and everybody elses opinion about the distinction between the avocado configs and test configs I mentioned above. If such a distinction is taken more seriously then a lot of this multi-job need becomes a need for better test configuration for particular types of tests and not a problem that is shared by all types of tests (and thus having to be job-wide solution).

[clebergnu]

The Job API IMO solves most of the command line expressiveness limitations, but, the only way to use the cartesian config or the Yaml to Mux varianter accross different tests with different parameters, is if they all agree to the same structure.

I guess what limits my understanding here is the fact that I rely on the same type of tests for all of my needs and often haven't really understood the need to use other test types. No matter what language a test is typed in, as long as it gets a config dictionary that could be tailored and scoped specifically to it either via config file or command line, I think everybody is happy. Correct me if I miss something of great importance but even the current discussion is not that much about some exotic differences in test structure and requirements as much as it is about this need to provide tailed configuration within some scope. The latter than sounds more like the job of a varianter than of the entire avocado command line.

I'm not sure I follow what you mean by test types, because you seem to imply that any test type can get a configuration as a dictionary. When I refer to test types here, I mean INSTRUMENTED, SIMPLE, TAP, etc. But, all of these inherit from avocado.Test and require the same interface. With VT test types, which I assume is what you're using, you're also indeed guaranteed to always have the same run() function definition.

Then, in the N(ext) Runner architecture, there's no common function signature, and test types are not required to inherit from a common type. This characteristic was drive by the fact that many projects have completely different "mini test frameworks" which are independent of each other. To accommodate those in a single Avocado Job, we need to have different sets of parameters coming from different places (or no places in some aspects).

This may be seen as a positive aspect for something like Avocado-VT, in which you do want to have a project-wide standard and the entry point into the test suite is the cartesian configuration itself, but it fails miserably on individual tests that have their individual parameters.

I don't understand this part quite well. The point of the Cartesian and any other varianter is precisely this - to provide individual parameters to tests. I guess you also mean that varianters also decide what final tests will be parsed in addition to providing them with unique configuration parameters which is a fair point.

What I mean is that on the Avocado-VT model, the Cartesian config determines the tests that will be generated in Avocado's test suite, and consequently executed, right?

This approach indeed allows one to determine that some tests will not have variants, while others will. And with that, what parameters tests will receive. But the Avocado core model is the opposite: variants are an addition to the selected tests, and do not influence the "bare test" (without variants) selection. So the selection of the "bare tests" needs to be done at some place, and we understood that "test suites" are a way to do it. Then, on top of them, variants (and their parameters) can be applied.

For instance, hooking a completely new test into Avocado-VT may require a lot of only this and no that just to get along with the variants and structure in place.

Not if the variant names are unique enough and with some thousand tests by now I have hardly had to use more then 1-2 restrictions like this.

Still it gets back to one of my original points, which is that the whole set of parameters need to be held together. On projects such as QEMU, with many "mini test frameworks" of their own, this is not practical. It's also not practical if one is attempting to create a "Integration Job" with Avocado that includes different software packages (and their also different "mini test frameworks").

Also, other variants such as the CIT perform at their best while creating variants with parameters that are really relevant to a test. Adding the ~200 key/values as input to CIT where a test will only use a few is a huge overkill.

The way the variants are defined in a Cartesian or YAML2MUX varianter can go anywhere from few to many parameters and this strictly depends on the use cases so I am not sure where the 200 key/value pairs come from. I guess such parameter redundancies could be attributed to limitations in some of the varianters. If you mean this in particular as something which is solved via shared parameter scopes across tests (here coined test suites) then for me these remains just test variant scopes and thus subject of the choice of varianter again.

What I mean by the 200 key/value pairs is what you get on an Avocado-VT test under Test parameters:, such as:

5:38:51 DEBUG| Test parameters:
15:38:51 DEBUG|     _name_map_file = {'machines.cfg': 'i440fx', 'subtests.cfg': '(subtest=io-github-autotest-qemu).(subtest=boot)', 'host.cfg': 'HostCpuFamily.skylake.HostCpuVersion.i7-6820HQ.HostCpuVendor.intel.Host.Fedora.m31.u0.Host_arch_x86_64', 'guest-os.cfg': 'Guest.Linux.JeOS.27.x86_64', 'guest-hw.cfg': 'bridge.default_bios.no_virtio_rng.(image_backend=filesystem).no_9p_export.smallpages.no_pci_assignable.qcow2.virtio_scsi.smp2.virtio_net', 'tests.cfg': 'qemu_kvm_jeos_quick'}
15:38:51 DEBUG|     _short_name_map_file = {'machines.cfg': 'i440fx', 'subtests.cfg': 'io-github-autotest-qemu.boot', 'host.cfg': 'HostCpuFamily.skylake.HostCpuVersion.i7-6820HQ.HostCpuVendor.intel.Host.Fedora.m31.u0.Host_arch_x86_64', 'guest-os.cfg': 'Guest.Linux.JeOS.27.x86_64', 'guest-hw.cfg': 'bridge.default_bios.no_virtio_rng.filesystem.no_9p_export.smallpages.no_pci_assignable.qcow2.virtio_scsi.smp2.virtio_net', 'tests.cfg': 'qemu_kvm_jeos_quick'}
15:38:51 DEBUG|     auto_cpu_model = yes
15:38:51 DEBUG|     backup_dir = images/
15:38:51 DEBUG|     backup_image = no
15:38:51 DEBUG|     backup_image_before_testing = yes
15:38:51 DEBUG|     backup_image_on_check_error = no
15:38:51 DEBUG|     boot_menu = off
15:38:51 DEBUG|     boot_once = c
...
15:38:51 DEBUG|     cpu_model_flags = VALUE_SUITABLE_TO_AVOCADO_VT_TESTS

So, let's assume that:

• we have a single set of parameters (like the ~200s we get in a stock Avocado-VT boot test),
• they are Avocado proper parameters (that is, they'll be visible in every test type)

In a "varianter driven" world (like the cartesian config approach on Avocado-VT) if I were to include, say, a QEMU custom and previously existing check_cpu_model_flags.py, I'd need to be aware if that test will misbehave because of any of those key/values. Or, I would need to "filter out" the variations that I don't want for check_cpu_model_flags.py and/or reset the variables.

In a "test suite driven" world, I simply isolate check_cpu_model_flags.py and give it their proper variants (and thus parameters).

Finally, the implementation for test runners that we have right now have the responsibility of running one test suite. It means that, with the Job API, we can one job configured to run one suite on the local system, while another somewhere else, and still present a unified result. I think the possibilities are endless and the complexity added here is small when compared to the use cases that we can solve straight away.

So to rephrase this advantage (to see if I understand it correctly), you mean that with a multi-suite job we can support test running also with multiple runners (and thus spawners/containers/platforms)?

Yes! And it should not be limited to just runners, but you may want to have, say, sysinfo collection for functional tests but not for unittests. In theory, there should be no limits what is possible to be different inside a test suite, as long as the test suite is involved in configuring/running that feature.

Variant scopes could take care of different platforms of execution, test types, etc. so long as there is a unified way to pass the output of a varianter (e.g. list of dictionaries with parameters for each test in JSON or some other standard format) to a runner or a run managing component.

OK, I think I understand what you mean by variants scopes now, and I agree. But the scoping of variants (and other configuration, such as runners described earlier) was the "scoping" approach we came up with.

I think most of this boils down to correctly separating test configuration originating from avocado config files and command line with test configuration originating from test varianters. The desire to provide parameters of various sharing scopes to tests that could go down to the individual test seems really familiar implementing half of a varianter into the avocado settings.

I think a varianter could actually be used to generate different test suite configurations, but this is a quite advanced and creative use case. For the general users, we want to let them quickly specify the scope of parameters/variants to a group of tests, and I believe the approach we chose here makes it straightforward and extensible.

Adapting an avocado plugin to the new settings made me wonder about this too: how much are we differentiating between configuration set in config files of special format (e.g. YAML or Cartesian) and configuration set in the default (empty or not) avocado configs? There is a similar confusion in Avocado VT as well where one would define e.g. default Qemu binary in both base.cfg and the vt.conf. Is there a clear line where one can say "this configures a test" and "this configures the plugins"? Because it seems many of us are confused about the current presence of dual configuration.

I absolutely agree it's not always clear what is a "framework configuration" and what is a "test parameter". With Python based tests, one can do (but shouldn't):

from avocado.core.settings import settings

And really mess up this separation of responsibilities. And a lot of the avocado_vt/plugins/ code in Avocado-VT "violates" this separation the other way around: pushing configuration and command line parameters into the test parameters.

I've recently eliminated from the varianters interface the concept of default_params, which allowed plugins to cross this line (see https://github.com/avocado-framework/avocado/pull/4042). Its motivation was having a plugin such as the deceased "avocado-virt" to have a say on which QEMU binary would be used by default by tests... again, crossing that line.

Now, we expect that those problems would be solved by the Job API and an (upcoming) saner test parameter definition. I'll try to document ASAP what I think of the current and future test parameter interface.

[pevogam]

I'm not sure I follow what you mean by test types, because you seem to imply that any test type can get a configuration as a dictionary. When I refer to test types here, I mean INSTRUMENTED, SIMPLE, TAP, etc. But, all of these inherit from avocado.Test and require the same interface. With VT test types, which I assume is what you're using, you're also indeed guaranteed to always have the same run() function definition.

Yes, this is precisely what I imply. My starting point is something like the reason python objects have a __dict__ attributes as pretty much all of them could be described as a dictionary. This should then be true also for test configuration parameters which are a subtype of objects, perhaps even easier to imagine as a dictionary.

In a "varianter driven" world (like the cartesian config approach on Avocado-VT) if I were to include, say, a QEMU custom and previously existing check_cpu_model_flags.py, I'd need to be aware if that test will misbehave because of any of those key/values. Or, I would need to "filter out" the variations that I don't want for check_cpu_model_flags.py and/or reset the variables.

Yes, from a varianter perspective however this is fine because the config could be defined in a way without redundant variables to begin with. The implementation of the VT tests indeed has some of its limitations of parameters slipping through. But yes, a non-varianter approach where test files (scripts, binaries, etc.) are considered as the final tests is simpler for most. For me and for many varianter users these test files are a mere reusable asset as we could have multiple variants and possibly very different "tests" (as the loader output entities) reusing the same code as a resource.

I think most of this boils down to correctly separating test configuration originating from avocado config files and command line with test configuration originating from test varianters. The desire to provide parameters of various sharing scopes to tests that could go down to the individual test seems really familiar implementing half of a varianter into the avocado settings.

I think a varianter could actually be used to generate different test suite configurations, but this is a quite advanced and creative use case. For the general users, we want to let them quickly specify the scope of parameters/variants to a group of tests, and I believe the approach we chose here makes it straightforward and extensible.

I agree.

So the two main conclusions to draw here are that:

1) The line of which parameters belong to the core avocado operations (for me these are the avocado settings) and which parameters belong to tests, test sets (e.g. test suites in the sense of the job API right now), and even test environments is very blurry at the moment and would need some long term clearing out. It still seems to me that some of the approaches above are trying to pull test-specific and test-set-specific configuration (incl. job configuration) to the general avocado configuration.

2) The second main point I draw from the conclusions above is that there is a clash of two different perspectives - that of a varianter world and that of a WYSIWYG test files world. The first is a configuration-first approach and the second is a file-first approach. Let's see if knowing this, in the future we could come up with a general abstraction model that handles both cases well and doesn't reimplement the functionality of one into the other.

[beraldoleal]

Hi @pevogam, sorry for the long delay to reply here.

Hi all, looking at some of the PRs related to this and the discussion here and somewhat from an external point of view I was wondering why is all of this extra complexity needed?

Besides the @clebergnu replies, I believe that an important aspect here is consistency, usability and, learning curve when using Avocado.

You might be already familiar with some Avocado internals and for you might be easier keeping doing this way. But IMO for new users, it is much easier to understand this by simply looking at a method signature and a dict structure. Having the need to learning how some specific plugin works, just to describe the test suites, it is a little painful IMO.

Yes, I understand your concerns about the "complexity" of the solutions here. And I would like to apologize because I didn't update this issue properly. My fault. During the implementation and some discussions on the PR, we decided to go with a different approach of proposed here, just to keep things consistent (but with the comments here in mind).

I tried to explain the main points on this thread https://github.com/avocado-framework/avocado/pull/4041#discussion_r463876297 and also converted the outcome solution into Job() and TestSuite() classes docstrings.

Not sure if you saw that, but I hope that will be clear and less "complex". Please, let us know what you think about it.

[beraldoleal]

Hi @pevogam,

Since the PR was already merged, please, let me know if we can close this issue or not. Maybe we need to move this discussion to another issue.

[clebergnu]

Given that the feature is implemented, I say this issue needs to be closed. Discussions on further (and different) development implementation can reference this, but should be its own new issue IMO, so I'm closing this one.

Thanks everyone for the ideas and feedback!

[pevogam]

Seems like my initial reply to https://github.com/avocado-framework/avocado/issues/4019#issuecomment-674042985 didn't send properly here but yes, it was that we can close this as well and see where the new test suite API gets us. I do agree that there should be a good learning curve for newcomers, my point was mostly about the way we provide this learning curve and that we don't reimplement already abstracted functionality in a separate incompatible way. Let's see what we come up with later on.