[OE-core] [PATCH 1/4] oeqa/core/runner: write testresult to json files

[Yeoh, Ee Peng]

Hi Richard

Current codes does load existing testresult json file if it exist, then it will write the new testresult into it based on the result_id.
> +    def _get_testresults(self, write_dir):
> +        testresults = {}
> +        file = os.path.join(write_dir, self.testresult_filename)
> +        if os.path.exists(file):
> +            with open(file, "r") as f:
> +                testresults = json.load(f)
> +        return testresults

I did have the same thinking on if we can have a common function to manage configuration and result_id or let individual test classes to manage it, in the end, the thinking was configuration/result_id were really responsibility of each test classes, where the json helper class inside runner shall not have the knowledge or know-how on configuration/result_id. Thus the decision in the end was to make json helper class responsibility as simple as to consume the configuration, results, result_id information provided by the test classes. Hope this explain the reason behind current design. 

Thank you very much for your attention & knowledge sharing! 

Thanks,
Yeoh Ee Peng

-----Original Message-----
From: richard.purdie at linuxfoundation.org [mailto:richard.purdie at linuxfoundation.org] 
Sent: Monday, October 22, 2018 4:32 PM
To: Yeoh, Ee Peng <ee.peng.yeoh at intel.com>; openembedded-core at lists.openembedded.org
Subject: Re: [OE-core] [PATCH 1/4] oeqa/core/runner: write testresult to json files

Hi Ee Peng,

Thanks, this is looking good, there is still one small tweak needed below.

On Mon, 2018-10-22 at 14:54 +0800, Yeoh Ee Peng wrote:
> As part of the solution to replace Testopia to store testresult, OEQA 
> need to output testresult into single json file, where json testresult 
> file will be stored in git repository by the future 
> test-case-management tools.
> 
> The json testresult file will store more than one set of results, 
> where each set of results was uniquely identified by the result_id.
> The result_id would be like "runtime-qemux86-core-image-sato", where 
> it was a runtime test with target machine equal to qemux86 and running 
> on core-image-sato image. The json testresult file will only store the 
> latest testresult for a given result_id. The json testresult file 
> contains the configuration (eg. COMMIT, BRANCH, MACHINE, IMAGE), 
> result (eg. PASSED, FAILED, ERROR), test log, and result_id.
> 
> Based on the destination json testresult file directory provided, it 
> could have multiple instances of bitbake trying to write json 
> testresult to a single testresult file, using locking a lockfile 
> alongside the results file directory to prevent races.
> 
> Also the library class inside this patch will be reused by the future 
> test-case-management tools to write json testresult for manual test 
> case executed.
> 
> Signed-off-by: Yeoh Ee Peng <ee.peng.yeoh at intel.com>
> ---
>  meta/lib/oeqa/core/runner.py | 40 
> +++++++++++++++++++++++++++++++++++++++-
>  1 file changed, 39 insertions(+), 1 deletion(-)
> 
> diff --git a/meta/lib/oeqa/core/runner.py 
> b/meta/lib/oeqa/core/runner.py index f1dd080..82463cf 100644
> --- a/meta/lib/oeqa/core/runner.py
> +++ b/meta/lib/oeqa/core/runner.py
> @@ -249,3 +256,34 @@ class OETestRunner(_TestRunner):
>              self._list_tests_module(suite)
>  
>          return OEListTestsResult()
> +
> +class OETestResultJSONHelper(object):
> +
> +    testresult_filename = 'testresults.json'
> +
> +    def _get_testresults(self, write_dir):
> +        testresults = {}
> +        file = os.path.join(write_dir, self.testresult_filename)
> +        if os.path.exists(file):
> +            with open(file, "r") as f:
> +                testresults = json.load(f)
> +        return testresults
> +
> +    def _create_json_testresults_string(self, result_id, test_result, configuration, write_dir):
> +        testresults = self._get_testresults(write_dir)
> +        testresult = {'configuration': configuration,
> +                      'result': test_result}
> +        testresults[result_id] = testresult
> +        return json.dumps(testresults, sort_keys=True, indent=4)
> +
> +    def _write_file(self, write_dir, file_name, file_content):
> +        file_path = os.path.join(write_dir, file_name)
> +        with open(file_path, 'w') as the_file:
> +            the_file.write(file_content)
> +
> +    def dump_testresult_file(self, result_id, test_result, configuration, write_dir):
> +        bb.utils.mkdirhier(write_dir)
> +        lf = bb.utils.lockfile(os.path.join(write_dir, 'jsontestresult.lock'))
> +        json_testresults = self._create_json_testresults_string(result_id, test_result, configuration, write_dir)
> +        self._write_file(write_dir, self.testresult_filename, json_testresults)
> +        bb.utils.unlockfile(lf)

Before we write out the file we need to load in any existing data so we effectively append to the data. I think if we do that this patch should be ready to merge.

I did also wonder if we need a common configuration function rather than duplicating the code into each of the test classes.

Cheers,

Richard