Add support for getting test results as json on the job json api #1424

foursixnine · 2017-08-09T08:59:04Z

This allows to be able to use the json api to get not only a job information but also the details if there are any

codecov · 2017-08-09T09:11:01Z

Codecov Report

Merging #1424 into master will increase coverage by <.01%.
The diff coverage is 100%.

@@            Coverage Diff             @@
##           master    #1424      +/-   ##
==========================================
+ Coverage   88.44%   88.44%   +<.01%     
==========================================
  Files         105      105              
  Lines        7951     7956       +5     
==========================================
+ Hits         7032     7037       +5     
  Misses        919      919

Impacted Files	Coverage Δ
lib/OpenQA/WebAPI/Controller/Test.pm	`98.17% <ø> (-0.11%)`	⬇️
lib/OpenQA/Schema/Result/Jobs.pm	`90.73% <100%> (+0.03%)`	⬆️
lib/OpenQA/Utils.pm	`91.12% <100%> (+0.47%)`	⬆️
lib/OpenQA/WebAPI/Controller/API/V1/Job.pm	`79.39% <100%> (+0.08%)`	⬆️
lib/OpenQA/WebAPI.pm	`92.28% <100%> (+0.02%)`	⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 92f4fa1...b1d5b82. Read the comment docs.

coolo · 2017-08-09T17:04:18Z

t/api/04-jobs.t

+    $t->get_ok('/api/v1/jobs/99926')->status_is(200);
+    $t->json_is('/job/testresults' => undef, 'Test details are not present');
+
+    $t->get_ok('/api/v1/jobs/99926?details=1')->status_is(200);


I don't like that this is a param. This should be a documented route as in jobs/99926/details

kraih · 2017-08-11T15:56:27Z

lib/OpenQA/Schema/Result/Jobs.pm

@@ -582,6 +583,10 @@ sub to_hash {
    if ($args{deps}) {
        $j = {%$j, %{$job->deps_hash}};
    }
+    if ($args{details}) {
+        my $testresults = read_test_modules($job);
+        $j = {%$j, testresults => $testresults};


Wouldn't $j->{testresults} = $testresults; or $j->{testresults} = read_test_modules($job); work just as well?

okurz · 2017-08-15T12:36:22Z

lib/OpenQA/Schema/Result/Jobs.pm

@@ -23,7 +23,8 @@ use JSON;
 use Fcntl;
 use DateTime;
 use db_helpers;
-use OpenQA::Utils qw(log_debug log_info log_warning parse_assets_from_settings locate_asset send_job_to_worker);
+use OpenQA::Utils
+  qw(log_debug log_info log_warning parse_assets_from_settings locate_asset send_job_to_worker read_test_modules);


I guess a line break within the 'qw()' part would look nicer

I like to use this syntax.

use OpenQA::Utils ( qw(log_debug log_info log_warning parse_assets_from_settings locate_asset), qw(send_job_to_worker read_test_modules); );

sure, could work as well :-)

Actually we could also just use tags too, as right now we are exporting 40 of the 42 routines in the OpenQA::Utils, having :test_handling or :logging or :scheduling etc to group a bit might come handy... or simply start extracting from Utils as proposed by @okurz below

I'm going with @kraih's proposal

use OpenQA::Utils ( qw(log_debug log_info log_warning), qw(parse_assets_from_settings locate_asset), qw(send_job_to_worker read_test_modules) );

Export tags seem like a good idea.

okurz · 2017-08-15T12:37:32Z

lib/OpenQA/Utils.pm

@@ -649,6 +650,48 @@ sub find_job {
    return $job;
 }

+sub read_test_modules {


I am not so sure if the "utils" file is really the right one for such route specific implementation. Also the file "Utils.pm" is getting a bit big. Anyone has an idea for a better location for a function used both in the webUI route as well as the API one?

Made more sense to have it in the Utils module, pretty much like find_job does. But I agree that Utils is growing up a bit.

okurz · 2017-08-15T12:38:58Z

lib/OpenQA/WebAPI.pm

@@ -343,7 +343,8 @@ sub startup {
    $api_ro->post('/jobs/restart')->name('apiv1_restart_jobs')->to('job#restart');

    my $job_r = $api_ro->route('/jobs/:jobid', jobid => qr/\d+/);
-    $api_public_r->route('/jobs/:jobid', jobid => qr/\d+/)->get('/')->name('apiv1_job')->to('job#show');
+    $api_public_r->route('/jobs/:jobid', jobid => qr/\d+/)->name('apiv1_job')->to('job#show');
+    $api_public_r->route('/jobs/:jobid/details', jobid => qr/\d+/)->name('apiv1_job')->to('job#show', details => 1);


Wasn't our idea to use the .json render route instead of an explicit API route? Or maybe even better to add the same to both?

We already had the API route, I just added the details part. Adding support to render it at /tests/:jobid/details would have been only redundant when what we actually want is the pure json data.

I'm not sure I get it but nevermind

okurz · 2017-08-28T12:57:20Z

LGTM

foursixnine · 2017-09-27T15:43:13Z

t/api/04-jobs.t

+    $t->get_ok('/api/v1/jobs/99963/details')->status_is(200);
+    $t->json_has('/job/testresults/0', 'Test details are there');
+    $t->json_is('/job/assets/hdd/0',           => 'hdd_image.qcow2', 'Job has hdd_image.qcow2 as asset');
+    $t->json_is('/job/testresults/0/category', => 'installation',    'Job category is "installation"');


@mudler You will be pleased :) Actually it made sense to add the test, just in case

This allows to be able to use the json api to get not only a job information but also the details if there are any Add tests to get test results from the job json api

commit b5f2b56 Merge: 92f4fa1 b1d5b82 Author: Santiago Zarate <santiago@zarate.net.ve> AuthorDate: Wed Sep 27 21:49:56 2017 +0200 Commit: GitHub <noreply@github.com> CommitDate: Wed Sep 27 21:49:56 2017 +0200 Merge pull request #1424 from foursixnine/feature/json_api/job_details Add support for getting test results as json on the job json api

AdamWill · 2017-10-10T14:43:00Z

note, I've been using e.g.:

https://openqa.fedoraproject.org/api/v1/jobs?ids=70581

instead of:

https://openqa.fedoraproject.org/api/v1/jobs/70581

because the former includes individual module results but the latter doesn't. How does this compare with that? I can't quite grok precisely what 'details' this shows...

coolo · 2017-10-11T05:57:53Z

details*json as in all needle matches - the epic is https://progress.opensuse.org/issues/20526

foursixnine changed the title ~~Add support for getting test results as json Move read_test_modules to OpenQA::Utils~~ Add support for getting test results as json on the job json api Aug 9, 2017

coolo reviewed Aug 9, 2017

View reviewed changes

foursixnine force-pushed the feature/json_api/job_details branch 3 times, most recently from 0087d43 to 2e53269 Compare August 11, 2017 08:21

kraih reviewed Aug 11, 2017

View reviewed changes

okurz reviewed Aug 15, 2017

View reviewed changes

foursixnine force-pushed the feature/json_api/job_details branch from 2e53269 to 60f7586 Compare August 28, 2017 12:05

foursixnine force-pushed the feature/json_api/job_details branch from 60f7586 to 4f4e66b Compare August 29, 2017 12:34

foursixnine force-pushed the feature/json_api/job_details branch 2 times, most recently from 76c3dd8 to c27f500 Compare September 18, 2017 11:19

foursixnine force-pushed the feature/json_api/job_details branch from c27f500 to 405b00f Compare September 27, 2017 15:41

foursixnine commented Sep 27, 2017

View reviewed changes

foursixnine added 2 commits September 27, 2017 18:00

Move read_test_modules to OpenQA::Utils

1ed6abe

Add support for getting test results as json

b1d5b82

This allows to be able to use the json api to get not only a job information but also the details if there are any Add tests to get test results from the job json api

foursixnine force-pushed the feature/json_api/job_details branch from 405b00f to b1d5b82 Compare September 27, 2017 16:47

foursixnine merged commit b5f2b56 into os-autoinst:master Sep 27, 2017

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add support for getting test results as json on the job json api #1424

Add support for getting test results as json on the job json api #1424

foursixnine commented Aug 9, 2017 •

edited

codecov bot commented Aug 9, 2017 •

edited

coolo Aug 9, 2017

kraih Aug 11, 2017 •

edited

okurz Aug 15, 2017

kraih Aug 15, 2017

okurz Aug 15, 2017

foursixnine Aug 28, 2017 •

edited

kraih Aug 28, 2017

okurz Aug 15, 2017

foursixnine Aug 28, 2017

okurz Aug 15, 2017

foursixnine Aug 28, 2017

okurz Aug 28, 2017

okurz commented Aug 28, 2017

foursixnine Sep 27, 2017

mudler Sep 27, 2017

AdamWill commented Oct 10, 2017

coolo commented Oct 11, 2017

Add support for getting test results as json on the job json api #1424

Add support for getting test results as json on the job json api #1424

Conversation

foursixnine commented Aug 9, 2017 • edited

codecov bot commented Aug 9, 2017 • edited

Codecov Report

Choose a reason for hiding this comment

kraih Aug 11, 2017 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

foursixnine Aug 28, 2017 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

okurz commented Aug 28, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

AdamWill commented Oct 10, 2017

coolo commented Oct 11, 2017

foursixnine commented Aug 9, 2017 •

edited

codecov bot commented Aug 9, 2017 •

edited

kraih Aug 11, 2017 •

edited

foursixnine Aug 28, 2017 •

edited