[Asterisk-code-review] RFC: Add global pluggable modules. (testsuite[master])

[Mark Michelson]

Mark Michelson has posted comments on this change.

Change subject: RFC: Add global pluggable modules.
......................................................................


Patch Set 6:

> If I understand, every test that runs will load all pluggable
 > global module(s) correct? I'm not sure if I like that. Seems like
 > it could potentially cause problems for a test that doesn't even
 > make use of the module itself. Unless that is what you were
 > thinking when you talked about adding an opt-in option. Meaning a
 > particular global module won't load unless the test opts-in.
 > 

Correct. The expected use case would be that a test plan would be set up to load a specific global module (or modules, perhaps), and would also specify a certain tag that tests must have specified in their test-config.yaml. The opt-in part would be a separate commit from this one.

 > I wonder too if the module itself really needs to be global or just
 > the configuration? If we are going to have to have each test opt-in
 > why not just make it a regular plugggable module. Each test could
 > then specify specific configuration information if it needed too.
 > If none existed it could pull the configuration from the global
 > config by default.


There are a couple of reasons for not doing the configuration in the test itself:
1) It's not easy to conditionally load a pluggable module in a specific test. So for instance, in this case, we'd want to do some test runs where the configuration is converted to realtime, but we'd want to do other test runs where the test is run exactly like it is. By letting global configuration determine whether the pluggable module is used or not, it allows different test runs to dictate whether the module is loaded.
2) The details for the database configuration are going to be environment-specific. Information like the DSN to connect to, the database username and password, the hostname and port of the database, are all going to depend on where the test is run. By changing the configuration at the global level, it means not having to alter dozens/hundreds of tests to fit a specific environment.
3) For a single test run, the database options are always going to be the same, no matter the test. Specifying configuration globally makes sense in a case like this. Since the configuration pertains to the pluggable module itself, it only makes sense to have the module be defined at the global level as well.

 > 
 > Even with some kind of opt-in mechanism don't we still need a way
 > to run the tests in different ways (one with config files the other
 > realtime)? Without a command line option how are we going to tell
 > the testsuite to run the tests using a different back end? Unless I
 > am misunderstanding something.

One thing to remember is that the top-level test-config.yaml starts with an option that essentially says "Use section 'blah' as my global configuration". By default, the testsuite uses the config-standard section that has nothing in it. What you would do for test runs is to have some test runs use the config-standard section. These runs would run the test using configuration files, just as is done now. Other test runs would use a realtime configuration section (I put a sample one in this commit called config-realtime). These test runs would have the configuration files turned into database data instead. So by changing a line in the top-level test-config.yaml  file, the test run can go from a standard test run to a realtime test run.

-- 
To view, visit https://gerrit.asterisk.org/1803
To unsubscribe, visit https://gerrit.asterisk.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Ieec1b1cfa48cadab108c4ab65122ce36ab697e4e
Gerrit-PatchSet: 6
Gerrit-Project: testsuite
Gerrit-Branch: master
Gerrit-Owner: Mark Michelson <mmichelson at digium.com>
Gerrit-Reviewer: Anonymous Coward #1000019
Gerrit-Reviewer: Ashley Sanders <asanders at digium.com>
Gerrit-Reviewer: Joshua Colp <jcolp at digium.com>
Gerrit-Reviewer: Kevin Harwell <kharwell at digium.com>
Gerrit-Reviewer: Mark Michelson <mmichelson at digium.com>
Gerrit-Reviewer: Matt Jordan <mjordan at digium.com>
Gerrit-HasComments: No