Test infrastructure should respect the root_dir and use properly isolated configuration trees

[baby-gnu]

Description of Issue/Question

Hello.

Trying to develop #49140 for #14613 (and related #7883, #32578), I figured out that the test infrastructure does not define and use proper root trees.

1. the configuration is transplanted to a root_dir by tests.integration.TestDaemon.transplant_configs
   • every paths in the configuration files are prefixed with /tmp/salt-tests-tmpdir/rootdir/
2. the tests.support.mixins.AdaptedConfigurationTestCaseMixin load the configuration files generated at the previous step
   • it changes the root_dir to a temporary directory /tmp/tmpXXXXXX/tmp/ and apply the new configuration

The actual test suite does not choke on this because salt accepts to use paths outside root_dir since not all options are prefixed with root_dir.

But in combination with my work at #49140, it results in the paths to be defined as /tmp/tmpXXXXXX/tmp/salt-tests-tmpdir/rootdir/ which is completely wrong and lots of tests fails because of that, like:

10:58:04           -> unit.fileserver.test_roots.RootsTest.test_dir_list  ......................
10:58:04        Traceback (most recent call last):
10:58:04          File "/tmp/kitchen/testing/tests/unit/fileserver/test_roots.py", line 158, in test_dir_list
10:58:04            self.assertIn('empty_dir', ret)
10:58:04        AssertionError: u'empty_dir' not found in []

I propose the following in preparation for salt to strictly respects the root_dir as everything must be under this root_dir:

1. generate a pristine root_dir
   • I proposed to define tests.integration.TestDaemon.generate_new_config for that purpose
   • create all the salt-needed directory tree and configuration files (like previously by tests.integration.TestDaemon.transplant_configs) but they are never used directly
   • copy files from tests/integration/files to the proper place under the pristine root_dir
   • replace the pristine root_dir path prefix from all the paths in the configuration files with / (or use any kind of placeholder to make sure it will raise if the replacement is not done properly)
2. duplicate the configuration tree each time we need to load a new configuration
   • when transplanting the configuration by tests.integration.TestDaemon.transplant_configs
   • when defining a temporary configuration with tests.support.mixins.AdaptedConfigurationTestCaseMixin
   • copy the pristine root_dir tree to the new root_dir
   • replace the placeholder prefix with the path to the new root_dir (the proposal Feature/expand all paths #49140 will make this step obsolete by prefixing all the paths with this new root_dir)
   • load the configuration with the new root_dir

Setup

N/A

Steps to Reproduce Issue

Run the integration tests.

Versions Report

salt --versions-report
Salt Version:
           Salt: 2018.3.3
 
Dependency Versions:
           cffi: Not Installed
       cherrypy: Not Installed
       dateutil: 2.6.1
      docker-py: Not Installed
          gitdb: Not Installed
      gitpython: Not Installed
          ioflo: Not Installed
         Jinja2: 2.10
        libgit2: 0.26.0
        libnacl: Not Installed
       M2Crypto: Not Installed
           Mako: Not Installed
   msgpack-pure: Not Installed
 msgpack-python: 0.5.6
   mysql-python: Not Installed
      pycparser: Not Installed
       pycrypto: 2.6.1
   pycryptodome: Not Installed
         pygit2: 0.26.2
         Python: 3.6.7 (default, Oct 22 2018, 11:32:17)
   python-gnupg: 0.4.1
         PyYAML: 3.12
          PyZMQ: 16.0.2
           RAET: Not Installed
          smmap: Not Installed
        timelib: Not Installed
        Tornado: 4.5.3
            ZMQ: 4.2.5
 
System Versions:
           dist: Ubuntu 18.04 bionic
         locale: UTF-8
        machine: x86_64
        release: 4.15.0-43-generic
         system: Linux
        version: Ubuntu 18.04 bionic

[garethgreenaway]

@saltstack/team-core Thoughts?

[waynew]

I'm definitely a fan of the idea - I don't know enough about the implications or implementation to be able to form a strong opinion. But generally speaking, I think it's good if tests can run using a specific directory.

[stale]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

If this issue is closed prematurely, please leave a comment and we will gladly reopen the issue.

[baby-gnu]

This issue definitely should stay open and be addressed one day.

Regards.

[stale]

Thank you for updating this issue. It is no longer marked as stale.

[stale]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

If this issue is closed prematurely, please leave a comment and we will gladly reopen the issue.

[mchugh19]

Possibly in need of a feature request flag to placate stalebot

[stale]

Thank you for updating this issue. It is no longer marked as stale.