[RFC,01/16] patman: Use test_util to show test results

Message ID 20200706034203.2171077-2-sjg@chromium.org
State Accepted
Commit 2e9a0cdfa8456636392f24dcc47e3270bd4818b7
Headers show
Series
  • RFC: patman: Collect review tags and comments from Patchwork
Related show

Commit Message

Simon Glass July 6, 2020, 3:41 a.m.
This handles skipped tests correctly, so use it instead of the existing
code.

Signed-off-by: Simon Glass <sjg at chromium.org>
---

 tools/patman/main.py      | 8 ++------
 tools/patman/test_util.py | 6 +++---
 2 files changed, 5 insertions(+), 9 deletions(-)

Comments

Daniel Axtens July 6, 2020, 4:46 a.m. | #1
Hi Simon,

I can't see a cover letter so apologies if I've misunderstood something
basic, but this doesn't appear to apply to the patchwork tree - I'm
guessing the patchwork relevance is with regards to the last few patches
that (AFAICT) parse the patchwork web interface for information?

I haven't fully digested the patches (and I lack a lot of context) but
is there a reason the patchwork API isn't able to meet your needs here?
(And if so, could we extend it rather than having you parse the frontend?)

Regards,
Daniel

> This handles skipped tests correctly, so use it instead of the existing
> code.
>
> Signed-off-by: Simon Glass <sjg at chromium.org>
> ---
>
>  tools/patman/main.py      | 8 ++------
>  tools/patman/test_util.py | 6 +++---
>  2 files changed, 5 insertions(+), 9 deletions(-)
>
> diff --git a/tools/patman/main.py b/tools/patman/main.py
> index 28a9a26087..03668d1bb8 100755
> --- a/tools/patman/main.py
> +++ b/tools/patman/main.py
> @@ -25,6 +25,7 @@ from patman import patchstream
>  from patman import project
>  from patman import settings
>  from patman import terminal
> +from patman import test_util
>  from patman import test_checkpatch
>  
>  
> @@ -101,12 +102,7 @@ elif options.test:
>          suite = doctest.DocTestSuite(module)
>          suite.run(result)
>  
> -    # TODO: Surely we can just 'print' result?
> -    print(result)
> -    for test, err in result.errors:
> -        print(err)
> -    for test, err in result.failures:
> -        print(err)
> +    sys.exit(test_util.ReportResult('patman', None, result))
>  
>  # Called from git with a patch filename as argument
>  # Printout a list of additional CC recipients for this patch
> diff --git a/tools/patman/test_util.py b/tools/patman/test_util.py
> index aac58fb72f..0827488f33 100644
> --- a/tools/patman/test_util.py
> +++ b/tools/patman/test_util.py
> @@ -123,12 +123,12 @@ def ReportResult(toolname:str, test_name: str, result: unittest.TestResult):
>      for test, err in result.failures:
>          print(err, result.failures)
>      if result.skipped:
> -        print('%d binman test%s SKIPPED:' %
> -              (len(result.skipped), 's' if len(result.skipped) > 1 else ''))
> +        print('%d %s test%s SKIPPED:' % (len(result.skipped), toolname,
> +            's' if len(result.skipped) > 1 else ''))
>          for skip_info in result.skipped:
>              print('%s: %s' % (skip_info[0], skip_info[1]))
>      if result.errors or result.failures:
> -        print('binman tests FAILED')
> +        print('%s tests FAILED' % toolname)
>          return 1
>      return 0
>  
> -- 
> 2.27.0.212.ge8ba1cc988-goog
>
> _______________________________________________
> Patchwork mailing list
> Patchwork at lists.ozlabs.org
> https://lists.ozlabs.org/listinfo/patchwork
Daniel Axtens July 6, 2020, 4:50 a.m. | #2
Daniel Axtens <dja at axtens.net> writes:

> Hi Simon,
>
> I can't see a cover letter so apologies if I've misunderstood something
> basic, but this doesn't appear to apply to the patchwork tree - I'm
> guessing the patchwork relevance is with regards to the last few patches
> that (AFAICT) parse the patchwork web interface for information?

Ah, nevermind, the cover letter got caught in moderation. I've released it.

pwclient speaks the old, less documented XML-RPC API. We have a new
REST API which is much better documented, and is explorable
(e.g. https://patchwork.ozlabs.org/api/ and
https://patchwork.readthedocs.io/en/latest/api/rest/ )

> I haven't fully digested the patches (and I lack a lot of context) but
> is there a reason the patchwork API isn't able to meet your needs here?
> (And if so, could we extend it rather than having you parse the frontend?)

So these questions still stand but for the REST API.

Regards,
Daniel

>
> Regards,
> Daniel
>
>> This handles skipped tests correctly, so use it instead of the existing
>> code.
>>
>> Signed-off-by: Simon Glass <sjg at chromium.org>
>> ---
>>
>>  tools/patman/main.py      | 8 ++------
>>  tools/patman/test_util.py | 6 +++---
>>  2 files changed, 5 insertions(+), 9 deletions(-)
>>
>> diff --git a/tools/patman/main.py b/tools/patman/main.py
>> index 28a9a26087..03668d1bb8 100755
>> --- a/tools/patman/main.py
>> +++ b/tools/patman/main.py
>> @@ -25,6 +25,7 @@ from patman import patchstream
>>  from patman import project
>>  from patman import settings
>>  from patman import terminal
>> +from patman import test_util
>>  from patman import test_checkpatch
>>  
>>  
>> @@ -101,12 +102,7 @@ elif options.test:
>>          suite = doctest.DocTestSuite(module)
>>          suite.run(result)
>>  
>> -    # TODO: Surely we can just 'print' result?
>> -    print(result)
>> -    for test, err in result.errors:
>> -        print(err)
>> -    for test, err in result.failures:
>> -        print(err)
>> +    sys.exit(test_util.ReportResult('patman', None, result))
>>  
>>  # Called from git with a patch filename as argument
>>  # Printout a list of additional CC recipients for this patch
>> diff --git a/tools/patman/test_util.py b/tools/patman/test_util.py
>> index aac58fb72f..0827488f33 100644
>> --- a/tools/patman/test_util.py
>> +++ b/tools/patman/test_util.py
>> @@ -123,12 +123,12 @@ def ReportResult(toolname:str, test_name: str, result: unittest.TestResult):
>>      for test, err in result.failures:
>>          print(err, result.failures)
>>      if result.skipped:
>> -        print('%d binman test%s SKIPPED:' %
>> -              (len(result.skipped), 's' if len(result.skipped) > 1 else ''))
>> +        print('%d %s test%s SKIPPED:' % (len(result.skipped), toolname,
>> +            's' if len(result.skipped) > 1 else ''))
>>          for skip_info in result.skipped:
>>              print('%s: %s' % (skip_info[0], skip_info[1]))
>>      if result.errors or result.failures:
>> -        print('binman tests FAILED')
>> +        print('%s tests FAILED' % toolname)
>>          return 1
>>      return 0
>>  
>> -- 
>> 2.27.0.212.ge8ba1cc988-goog
>>
>> _______________________________________________
>> Patchwork mailing list
>> Patchwork at lists.ozlabs.org
>> https://lists.ozlabs.org/listinfo/patchwork
Simon Glass July 6, 2020, 2:52 p.m. | #3
Hi Daniel,

On Sun, 5 Jul 2020 at 22:50, Daniel Axtens <dja at axtens.net> wrote:
>
> Daniel Axtens <dja at axtens.net> writes:
>
> > Hi Simon,
> >
> > I can't see a cover letter so apologies if I've misunderstood something
> > basic, but this doesn't appear to apply to the patchwork tree - I'm
> > guessing the patchwork relevance is with regards to the last few patches
> > that (AFAICT) parse the patchwork web interface for information?
>
> Ah, nevermind, the cover letter got caught in moderation. I've released it.
>
> pwclient speaks the old, less documented XML-RPC API. We have a new
> REST API which is much better documented, and is explorable
> (e.g. https://patchwork.ozlabs.org/api/ and
> https://patchwork.readthedocs.io/en/latest/api/rest/ )
>
> > I haven't fully digested the patches (and I lack a lot of context) but
> > is there a reason the patchwork API isn't able to meet your needs here?
> > (And if so, could we extend it rather than having you parse the frontend?)
>
> So these questions still stand but for the REST API.

So use the REST API instead of the web page? That sounds fine to me.
Is it generally enabled on patchwork servers?

What is the status of pwclient? Is it dead? Is there a replacement?
I'd love to use a Python library if one exists.

Regards,
SImon
Daniel Axtens July 7, 2020, 1:09 a.m. | #4
Simon Glass <sjg at chromium.org> writes:

> Hi Daniel,
>
> On Sun, 5 Jul 2020 at 22:50, Daniel Axtens <dja at axtens.net> wrote:
>>
>> Daniel Axtens <dja at axtens.net> writes:
>>
>> > Hi Simon,
>> >
>> > I can't see a cover letter so apologies if I've misunderstood something
>> > basic, but this doesn't appear to apply to the patchwork tree - I'm
>> > guessing the patchwork relevance is with regards to the last few patches
>> > that (AFAICT) parse the patchwork web interface for information?
>>
>> Ah, nevermind, the cover letter got caught in moderation. I've released it.
>>
>> pwclient speaks the old, less documented XML-RPC API. We have a new
>> REST API which is much better documented, and is explorable
>> (e.g. https://patchwork.ozlabs.org/api/ and
>> https://patchwork.readthedocs.io/en/latest/api/rest/ )
>>
>> > I haven't fully digested the patches (and I lack a lot of context) but
>> > is there a reason the patchwork API isn't able to meet your needs here?
>> > (And if so, could we extend it rather than having you parse the frontend?)
>>
>> So these questions still stand but for the REST API.
>
> So use the REST API instead of the web page? That sounds fine to me.
> Is it generally enabled on patchwork servers?

I mean patman is your code so it's ultimately not my call :P But yes,
I'd strongly prefer you used the REST API! It is enabled on ozlabs.org
and kernel.org and has been for a while (~a couple of years).

> What is the status of pwclient? Is it dead? Is there a replacement?
> I'd love to use a Python library if one exists.

Stephen F is the expert on the client stuff, so I'm not going to make a
call on the status of pwclient. All I am confident to say is that I have
migrated to using 'git-pw' and I recommend others do so to too.

I'm not sure if a dedicated Python client library exists: the last time
I wanted to write a Python client app I found it simple enough to just
use the JSON that the API provides directly. But the place I'd start
with is git-pw.

Regards,
Daniel

> Regards,
> SImon
Stephen Finucane July 8, 2020, 5:42 p.m. | #5
On Tue, 2020-07-07 at 11:09 +1000, Daniel Axtens wrote:
> Simon Glass <sjg at chromium.org> writes:
> 
> > Hi Daniel,
> > 
> > On Sun, 5 Jul 2020 at 22:50, Daniel Axtens <dja at axtens.net> wrote:
> > > Daniel Axtens <dja at axtens.net> writes:
> > > 
> > > > Hi Simon,
> > > > 
> > > > I can't see a cover letter so apologies if I've misunderstood something
> > > > basic, but this doesn't appear to apply to the patchwork tree - I'm
> > > > guessing the patchwork relevance is with regards to the last few patches
> > > > that (AFAICT) parse the patchwork web interface for information?
> > > 
> > > Ah, nevermind, the cover letter got caught in moderation. I've released it.
> > > 
> > > pwclient speaks the old, less documented XML-RPC API. We have a new
> > > REST API which is much better documented, and is explorable
> > > (e.g. https://patchwork.ozlabs.org/api/ and
> > > https://patchwork.readthedocs.io/en/latest/api/rest/ )
> > > 
> > > > I haven't fully digested the patches (and I lack a lot of context) but
> > > > is there a reason the patchwork API isn't able to meet your needs here?
> > > > (And if so, could we extend it rather than having you parse the frontend?)
> > > 
> > > So these questions still stand but for the REST API.
> > 
> > So use the REST API instead of the web page? That sounds fine to me.
> > Is it generally enabled on patchwork servers?
> 
> I mean patman is your code so it's ultimately not my call :P But yes,
> I'd strongly prefer you used the REST API! It is enabled on ozlabs.org
> and kernel.org and has been for a while (~a couple of years).
> 
> > What is the status of pwclient? Is it dead? Is there a replacement?
> > I'd love to use a Python library if one exists.
> 
> Stephen F is the expert on the client stuff, so I'm not going to make a
> call on the status of pwclient. All I am confident to say is that I have
> migrated to using 'git-pw' and I recommend others do so to too.

pwclient is alive and won't be going anywhere, but it's still using the
now-deprecated XML-RPC API. Eventually this will move to the REST API,
but I just haven't found the time to do so yet.

> I'm not sure if a dedicated Python client library exists: the last time
> I wanted to write a Python client app I found it simple enough to just
> use the JSON that the API provides directly. But the place I'd start
> with is git-pw.

There's no patchwork SDK yet and git-pw is intended for use as a tool
rather than a library (the internal API isn't stable). However, the
Patchwork API itself is very trivial so requests should get you what
you want very easily.

Stephen

> Regards,
> Daniel
> 
> > Regards,
> > SImon

Patch

diff --git a/tools/patman/main.py b/tools/patman/main.py
index 28a9a26087..03668d1bb8 100755
--- a/tools/patman/main.py
+++ b/tools/patman/main.py
@@ -25,6 +25,7 @@  from patman import patchstream
 from patman import project
 from patman import settings
 from patman import terminal
+from patman import test_util
 from patman import test_checkpatch
 
 
@@ -101,12 +102,7 @@  elif options.test:
         suite = doctest.DocTestSuite(module)
         suite.run(result)
 
-    # TODO: Surely we can just 'print' result?
-    print(result)
-    for test, err in result.errors:
-        print(err)
-    for test, err in result.failures:
-        print(err)
+    sys.exit(test_util.ReportResult('patman', None, result))
 
 # Called from git with a patch filename as argument
 # Printout a list of additional CC recipients for this patch
diff --git a/tools/patman/test_util.py b/tools/patman/test_util.py
index aac58fb72f..0827488f33 100644
--- a/tools/patman/test_util.py
+++ b/tools/patman/test_util.py
@@ -123,12 +123,12 @@  def ReportResult(toolname:str, test_name: str, result: unittest.TestResult):
     for test, err in result.failures:
         print(err, result.failures)
     if result.skipped:
-        print('%d binman test%s SKIPPED:' %
-              (len(result.skipped), 's' if len(result.skipped) > 1 else ''))
+        print('%d %s test%s SKIPPED:' % (len(result.skipped), toolname,
+            's' if len(result.skipped) > 1 else ''))
         for skip_info in result.skipped:
             print('%s: %s' % (skip_info[0], skip_info[1]))
     if result.errors or result.failures:
-        print('binman tests FAILED')
+        print('%s tests FAILED' % toolname)
         return 1
     return 0