From c5503b3537b55dd458f86ad99a663857d1a3e135 Mon Sep 17 00:00:00 2001 From: Frantisek Hrbata Date: Wed, 2 Aug 2023 10:04:34 +0200 Subject: [PATCH] ci(tools): fix test_hints.py to run on windows 1. The original test, before hint modules support was added, used tempfile.NamedTemporaryFile in a way which is not supported on windows. It was having the file open, which the hints tried to read it, leading the EPERM exception. The docs[1] says this is not supported. Whether the name can be used to open the file a second time, while the named temporary file is still open, varies across platforms (it can be so used on Unix; it cannot on Windows) 2. The hint module component_requirements test used the idf.py directly, which is idf.py.exe on windows.Now it's starting idf.py through python. We could probably used shell=True, but this approach is used in other tests too. Anyway the test are now passing on windows. [1] https://docs.python.org/3/library/tempfile.html Signed-off-by: Frantisek Hrbata --- tools/test_idf_py/test_hints.py | 46 +++++++++++++++++++++++---------- 1 file changed, 32 insertions(+), 14 deletions(-) diff --git a/tools/test_idf_py/test_hints.py b/tools/test_idf_py/test_hints.py index 33e0ded6f26..d1164ff0a12 100755 --- a/tools/test_idf_py/test_hints.py +++ b/tools/test_idf_py/test_hints.py @@ -12,6 +12,13 @@ import yaml +try: + EXT_IDF_PATH = os.environ['IDF_PATH'] # type: str +except KeyError: + print(('This test needs to run within ESP-IDF environmnet. ' + 'Please run export script first.'), file=sys.stderr) + exit(1) + CWD = os.path.join(os.path.dirname(__file__)) ERR_OUT_YML = os.path.join(CWD, 'error_output.yml') @@ -23,21 +30,32 @@ class TestHintsMassages(unittest.TestCase): + def setUp(self) -> None: + self.tmpdir = tempfile.TemporaryDirectory() + def test_output(self) -> None: with open(ERR_OUT_YML) as f: error_output = yaml.safe_load(f) + + error_filename = os.path.join(self.tmpdir.name, 'hint_input') for error, hint in error_output.items(): - with tempfile.NamedTemporaryFile(mode='w') as f: + with open(error_filename, 'w') as f: f.write(error) - f.flush() - for generated_hint in generate_hints(f.name): - self.assertEqual(generated_hint, hint) + for generated_hint in generate_hints(f.name): + self.assertEqual(generated_hint, hint) + + def tearDown(self) -> None: + self.tmpdir.cleanup() class TestHintModuleComponentRequirements(unittest.TestCase): - def run_cmd(self, cmd: List[str]) -> str: - # Simple helper to run command and return it's stdout. - proc = run(cmd, capture_output=True, cwd=str(self.projectdir), text=True) + def run_idf(self, args: List[str]) -> str: + # Simple helper to run idf command and return it's stdout. + cmd = [ + sys.executable, + os.path.join(os.environ['IDF_PATH'], 'tools', 'idf.py') + ] + proc = run(cmd + args, capture_output=True, cwd=str(self.projectdir), text=True) return proc.stdout + proc.stderr def setUp(self) -> None: @@ -69,39 +87,39 @@ def test_component_requirements(self) -> None: # The main component uses component1.h, but this header is not in component1 public # interface. Hints should suggest that component1.h should be added into INCLUDE_DIRS # of component1. - output = self.run_cmd(['idf.py', 'app']) + output = self.run_idf(['app']) self.assertIn('Missing "component1.h" file name found in the following component(s): component1(', output) # Based on previous hint the component1.h is added to INCLUDE_DIRS, but main still doesn't # have dependency on compoment1. Hints should suggest to add component1 into main component # PRIV_REQUIRES, because foo.h is not in main public interface. - self.run_cmd(['idf.py', 'fullclean']) + self.run_idf(['fullclean']) component1cmake = self.projectdir / 'components' / 'component1' / 'CMakeLists.txt' component1cmake.write_text('idf_component_register(INCLUDE_DIRS ".")') - output = self.run_cmd(['idf.py', 'app']) + output = self.run_idf(['app']) self.assertIn('To fix this, add component1 to PRIV_REQUIRES list of idf_component_register call', output) # Add foo.h into main public interface. Now the hint should suggest to use # REQUIRES instead of PRIV_REQUIRES. - self.run_cmd(['idf.py', 'fullclean']) + self.run_idf(['fullclean']) maincmake = self.projectdir / 'main' / 'CMakeLists.txt' maincmake.write_text(('idf_component_register(SRCS "foo.c" ' 'REQUIRES esp_timer ' 'INCLUDE_DIRS ".")')) - output = self.run_cmd(['idf.py', 'app']) + output = self.run_idf(['app']) self.assertIn('To fix this, add component1 to REQUIRES list of idf_component_register call', output) # Add component1 to REQUIRES as suggested by previous hint, but also # add esp_psram as private req for component1 and add esp_psram.h # to component1.h. New the hint should report that esp_psram should # be moved from PRIV_REQUIRES to REQUIRES for component1. - self.run_cmd(['idf.py', 'fullclean']) + self.run_idf(['fullclean']) maincmake.write_text(('idf_component_register(SRCS "foo.c" ' 'REQUIRES esp_timer component1 ' 'INCLUDE_DIRS ".")')) (self.projectdir / 'components' / 'component1' / 'component1.h').write_text('#include "esp_psram.h"') component1cmake.write_text('idf_component_register(INCLUDE_DIRS "." PRIV_REQUIRES esp_psram)') - output = self.run_cmd(['idf.py', 'app']) + output = self.run_idf(['app']) self.assertIn('To fix this, move esp_psram from PRIV_REQUIRES into REQUIRES', output) def tearDown(self) -> None: