Fixtures: Fix bug in reset of empty_config fixture

[sphuber]

The empty_config fixture creates a completely independent configuration directory which allows test to manipulate the config and its contents, such as the profiles, for the duration of the test without affecting pre-existing config directories.

To accomplish this, the AIIDA_PATH environment variable, which indicates the location of the configuration directory, is set to a temporary directory. The fixture then calls the method aiida.manage.configuration.settings.set_configuration_directory which will set a number of global variables that define important directory and file locations within the configuration folder, for example the location of important daemon files.

The bug was that these global variables weren't reset after the end of the fixture. This means that operations performed by the daemon client, such as checking whether the daemon is running, which rely on these filepaths being set correctly, would fail. The solution therefore is to call set_configuration_directory once again, to revert the global variables to their previous values.

This same problem also occurred in the cache_aiida_path_variable fixture defined and used in tests/manage/configuration/test_config.py. An explicit call after the finally in the fixture is necessary to reset the global variables.

Finally, test_environment_variable_not_set set the value of the DEFAULT_AIIDA_PATH variable to a temporary directory, however, it never changed it back to the default ~. This would essentially cause the set_configuration_directory be ineffective if the AIIDA_PATH variable was never set before running the tests, as then the value of DEFAULT_AIIDA_PATH would be used, which was still the temporary directory of the test. This would break any following tests that rely on the global variables in the settings module to be properly set.

[ltalirz]

thanks @sphuber !

great that you managed to track this down to make the tests more reproducible.

I have a few minor suggestions for further improvement

[ltalirz]

by the way, if the goal is strictly to reconstruct the state before this fixture was called, then I guess in principle one should check whether the "current profile" was loaded before or not.

I see that the current Manager implementation does not provide information on whether that was actually the case.
It may make sense to add an

@property
def is_profile_loaded(self):
  return self._profile is not None

property in the Manager class (could also be used inside the manager implementation in some places)

[ltalirz]

thanks a lot @sphuber !

looks great to me, as well as easier to follow