Hi Richard, Thanks for sharing with us your inputs for this patch.
For the weird "_" prefixes, the 3 new methods were initially developed in “OESelftestTestContextExecutor” class to gather information needed for selftest to write json test result file, while the “OESelftestTestContextExecutor” class have the public interface such as "run" and "register_commands", the decision at that point was to use "_" for the 3 new methods to highlight that these new methods for was internal use and avoid confusion with the existing public interface. While porting these new methods to bbclass, I made the mistake and missed the fact that "_" was a weird practice and make no sense in bbclass. Thank you for sharing this with me, I will pay special attention and avoid this in future. For the identifiers for the configuration sections in the json files are not unique and results from multiple runs were being overwritten locally. This was done intentionally as the initial design of json test results file was for the QA usage. From QA usage, there could be multiple set of tests being executed for the same configuration, some of these test result sets may be invalid due to environments used, where QA only want the latest set of test that was valid. Thus this result in the decision to have the code that generate the unique key without timestamp, so that the json test results file will only store the latest valid result. Hope this explain the reason. Now that you shared with us the important of timestamp, we understand the responsibility of this json test results file was more than QA, where it was important to keep record for all executed tests. Hope this explain some reasons for the patch submitted. Thank you again for sharing your inputs with us! Best regards, Yeoh Ee Peng -----Original Message----- From: richard.pur...@linuxfoundation.org [mailto:richard.pur...@linuxfoundation.org] Sent: Monday, October 29, 2018 9:59 PM To: Yeoh, Ee Peng <ee.peng.y...@intel.com>; openembedded-core@lists.openembedded.org Subject: Re: [OE-core] [PATCH 1/4] oeqa/core/runner: write testresult to json files On Mon, 2018-10-29 at 10:44 +0000, richard.pur...@linuxfoundation.org wrote: > On Tue, 2018-10-23 at 06:39 +0000, Yeoh, Ee Peng wrote: > > I submitted the revised patches below. Sorry for the missing > > "version #" in the patch title. After this, I will add the "version > > #" into the patch title. > > > > Please let me know if any question or inputs. Thank you very much > > for your attention & sharing! > > Thanks for the changes. I was at a conference last week and then > became unwell. I wanted to have a further look and test the patches > before merging and I wanted a clearer head to do that. > > Unfortunately I found another problem. On my build machine, it shows: > > Traceback (most recent call last): > File "/media/build1/poky/scripts/oe-selftest", line 70, in <module> > ret = main() > File "/media/build1/poky/scripts/oe-selftest", line 57, in main > results = args.func(logger, args) > File "/media/build1/poky/meta/lib/oeqa/selftest/context.py", line > 289, in run > rc = self._internal_run(logger, args) > File "/media/build1/poky/meta/lib/oeqa/selftest/context.py", line > 248, in _internal_run > configuration = self._get_configuration(args) > File "/media/build1/poky/meta/lib/oeqa/selftest/context.py", line > 217, in _get_configuration > metadata = metadata_from_bb() > File "/media/build1/poky/meta/lib/oeqa/utils/metadata.py", line 42, > in metadata_from_bb > info_dict['layers'] = get_layers(data_dict['BBLAYERS']) > File "/media/build1/poky/meta/lib/oeqa/utils/metadata.py", line 81, > in get_layers > layer_dict[layer_name] = git_rev_info(layer) > File "/media/build1/poky/meta/lib/oeqa/utils/metadata.py", line 61, > in git_rev_info > from git import Repo, InvalidGitRepositoryError, NoSuchPathError > ModuleNotFoundError: No module named 'git' > > That is obviously easily fixed by installing the git module but it > does raise some questions, in particular, why we have two code paths > which do the same thing (one in metadata_scm.bbclass and one in > lib/oeqa/utils/metadata.py). > > It also means we've just added new module dependencies to oe-selftest > and the other test utilities which we don't test for anywhere or have > documented. Doing this last thing in M4 is bad. > > So this is going to need a little more thought... I've played with the patches today. The above can be addressed by using the same code as we use in metadata_scm in the oeqa metadata file as a quick fix. This all needs to be reworked in 2.7 after we sort out 2.6. I've a patch queued. I also noticed that: * We have pointless empty log entries in the json files * SDKs don't record which MACHINE built them (in the unique identifier or the configuration section) * the identifiers for the configuration sections in the json files are not unique and results from multiple runs were being overwritten locally * the patches call SDKMACHINE SDK_MACHINE which just confuses things * The layer metadata config was being squashed into a single entry with multiple contents, there is no good reason to do that in json, just leave the fields separate * The layer metadata was being obtained from different functions, potentially leading to different processing * The output data was not being placed in LOG_DIR * The functions still had weird "_" prefixes Since time is short, to fix these issues I'm going to include a set of tweaks for the patches. Cheers, Richard -- _______________________________________________ Openembedded-core mailing list Openembedded-core@lists.openembedded.org http://lists.openembedded.org/mailman/listinfo/openembedded-core
