Fix for module import issue

[pbaughman]

• Try a 'real' import of the test file name before we load the module from source.
  This prevents the source file loader from hiding the real module (if it exists)
  in the sys.modules cache

Signed-off-by: Pete Baughman [email protected]

Closes #357

[pbaughman]

@hidmic Here's the fix. It's more explanation than code

[wjwwood]

I guess this is ok? Seems like a hack, but maybe it will help someone avoid a rough edge, though I wouldn't expect launch to do this for you, so it may be surprising either way.

[pbaughman]

@wjwwood launch gives the file a fixed module name "python_launch_file" so unless someone makes a module with the same name, launch will not encounter the same problem.

I don't remember why 'launch' and 'launch_testing' don't use the same load_python_launch_file_as_module method. It's probably the result of launch_testing being developed against a frozen version of 'launch' back when it was apex_launchtest

An alternative fix would be to have launch_testing use the same method as launch to import the test file - maybe import it as python_launch_test instead of python_launch_file, but as mentioned in the issue that will change the names of the tests that get generated.

[hidmic]

Seems like a hack, but maybe it will help someone avoid a rough edge, though I wouldn't expect launch to do this for you, so it may be surprising either way.

It is a bit of a hack, and I agree that it might silently incur in unwanted side effects. I didn't know of a superior alternative though, for the reasons that @pbaughman already mentioned in #357.

Having said that, after digging a bit further into importlib documentation, there might be a way to import a module while preventing the interpreter from caching it in sys.modules. @pbaughman mind to try this (without actually caching anything)?

[pbaughman]

@hidmic I tried doing this:

def _load_python_file_as_module(test_module_name, python_file_path):                 
    """Load a given Python launch file (by path) as a Python module."""              

    spec = importlib.util.spec_from_file_location(test_module_name, python_file_path)
    return importlib.util.module_from_spec(spec)                                     

But now things are blowing up here: https://github.com/ros2/launch/blob/master/launch_testing/launch_testing/loader.py#L154

with an error "module lidar_integration has no attribute 'generate_test_description'

I think I've just reversed the problem. Before, it was looking in the test file for things that should be found in the real module. Now I think it's looking in the real module for things that should be found in the test file.

[pbaughman]

Maybe the correct fix here is "don't name your test files the same thing as a real module" and a better warning/error message when you do. . .

[pbaughman]

@hidmic Ah, no I was wrong. . . This does it:

def _load_python_file_as_module(test_module_name, python_file_path):                 
    """Load a given Python launch file (by path) as a Python module."""              
    # Taken from launch_testing to not introduce a weird dependency thing            
    spec = importlib.util.spec_from_file_location(test_module_name, python_file_path)
    module = importlib.util.module_from_spec(spec)                                   
    spec.loader.exec_module(module)                                                  
    return module                                                                    

[hidmic]

LGTM pending green CI

[hidmic]

CI (up to launch_testing, launch_testing_ros, test_communication)

• Linux
• Linux-aarch64
• macOS
• Windows

[hidmic]

Alright, all green. Going in!