Open PraveenPenguin opened 1 month ago
Sample run from misc test : before fix:
avocado run eatmemory.py
Fetching asset from eatmemory.py:EatMemory.test
JOB ID : 30f28ac7104b4f3ad3828ea73a43fa390c6985b1
JOB LOG : /mnt/rootlv/avocado-fvt-wrapper/results/job-2024-05-14T03.37-30f28ac/job.log
(1/1) eatmemory.py:EatMemory.test: STARTED
(1/1) eatmemory.py:EatMemory.test: ERROR: Command 'patch -p1 < eatmemory.py.data/eatmem_getch.patch' failed.\nstdout: b''\nstderr: b'/bin/sh: eatmemory.py.data/eatmem_getch.patch: No such file or directory\n'\nadditional_info: None (0.09 s)
RESULTS : PASS 0 | ERROR 1 | FAIL 0 | SKIP 0 | WARN 0 | INTERRUPT 0 | CANCEL 0
JOB HTML : /mnt/rootlv/avocado-fvt-wrapper/results/job-2024-05-14T03.37-30f28ac/results.html
JOB TIME : 32.03 s
after fix
avocado run eatmemory.py
Fetching asset from eatmemory.py:EatMemory.test
JOB ID : bbbea168b9ecfacd3f834ffa75ea24826729bacf
JOB LOG : /mnt/rootlv/avocado-fvt-wrapper/results/job-2024-05-14T03.24-bbbea16/job.log
(1/1) eatmemory.py:EatMemory.test: STARTED
(1/1) eatmemory.py:EatMemory.test: PASS (7.74 s)
RESULTS : PASS 1 | ERROR 0 | FAIL 0 | SKIP 0 | WARN 0 | INTERRUPT 0 | CANCEL 0
JOB HTML : /mnt/rootlv/avocado-fvt-wrapper/results/job-2024-05-14T03.24-bbbea16/results.html
JOB TIME : 41.08 s
@richtja can you please review this PR
Hi @PraveenPenguin even tho I understand that this will solve issue on misc test side I am not sure If we can make this change. The
get_data
is part of the avocado-instrumented core API and this change will affect all of our users, therefore we try to avoid such changes.Is there a possibility to solve this on misc test side? And if it is not, I would propose to change the API in a less influencing way like this:
get_data(self, filename, source=None, must_exist=True, abs_path=False)
@clebergnu what do you think?
@richtja thanks for input ,yes make more sense to add abs_path as argument and misc test side we can overwrite this having asigining to True
@richtja pushed changes , but CI is failing as
Log file "debug.log" content for test "functional-parallel-118-selftests/functional/basic.py:RunnerOperationTest.test_store_logging_stream_external" (FAIL):
[stdout] ======================================================================
[stdout] FAIL: test_store_logging_stream_external (basic.RunnerOperationTest)
[stdout] :avocado: dependency={"type": "ansible-module", "uri": "pip", "name": "matplotlib"}
functional-parallel-118-selftests/functional/basic.py:RunnerOperationTest.test_store_logging_stream_external: FAIL
[stdout] ----------------------------------------------------------------------
[stdout] Traceback (most recent call last):
[stdout] File "/home/runner/work/avocado/avocado/selftests/functional/basic.py", line 848, in test_store_logging_stream_external
[stdout] check_matplotlib_logs(os.path.join(log_dir, "full.log"))
[stdout] File "/home/runner/work/avocado/avocado/selftests/functional/basic.py", line 833, in check_matplotlib_logs
[stdout] self.assertIn("matplotlib __init__ L0337 DEBUG|", stream)
[stdout] AssertionError: 'matplotlib __init__ L0337 DEBUG|' not found in '2024-05-16 07:16:47,863 avocado.app human L0064 INFO | JOB ID :
that nothing to do with this patch i feel , looking to fix it but meanwhile can please review this patch and if looks good merge it as this is blocker to misc-test
@richtja pushed changes , but CI is failing as
Log file "debug.log" content for test "functional-parallel-118-selftests/functional/basic.py:RunnerOperationTest.test_store_logging_stream_external" (FAIL): [stdout] ====================================================================== [stdout] FAIL: test_store_logging_stream_external (basic.RunnerOperationTest) [stdout] :avocado: dependency={"type": "ansible-module", "uri": "pip", "name": "matplotlib"} functional-parallel-118-selftests/functional/basic.py:RunnerOperationTest.test_store_logging_stream_external: FAIL [stdout] ---------------------------------------------------------------------- [stdout] Traceback (most recent call last): [stdout] File "/home/runner/work/avocado/avocado/selftests/functional/basic.py", line 848, in test_store_logging_stream_external [stdout] check_matplotlib_logs(os.path.join(log_dir, "full.log")) [stdout] File "/home/runner/work/avocado/avocado/selftests/functional/basic.py", line 833, in check_matplotlib_logs [stdout] self.assertIn("matplotlib __init__ L0337 DEBUG|", stream) [stdout] AssertionError: 'matplotlib __init__ L0337 DEBUG|' not found in '2024-05-16 07:16:47,863 avocado.app human L0064 INFO | JOB ID :
that nothing to do with this patch i feel , looking to fix it but meanwhile can please review this patch and if looks good merge it as this is blocker to misc-test
Hi @PraveenPenguin yes this looks like an issue of the CI, I will look at that. And about the review, thank you for your changes, I just would like to get opinion from @clebergnu
@clebergnu can you please put your thought on this, as we are blocked on misc test side
Hi @PraveenPenguin ,
I tried to approach this "Test API" change with an open mind, but I'm still not convinced that the change makes sense. The use case, clearly does make sense, but I can not see why it should be part of that API.
If you look at the other parameters, they are not things that can be done outside of the responsibility of the method. In the case of converting non-absolute paths to absolute paths, that can be easily done outside of it.
For instance, what would happen if that API had the behavior of returning absolute paths, and one user wanted relative paths instead? IMO, it would be natural to simply use os.path.relpath() instead of changing the API.
If you have some other reasons or points I'm missing, please let me know.
@clebergnu Sure, it looks ok but why this sudden change even though I am not able to get commit which yields this as earlier it was working
As in misc test repo apply patch is failing which part of data dir as utility return relative path this patch address as it return absolute file path (file or dir)