psss / did

What did you do last week, month, year?
https://did.readthedocs.io/
GNU General Public License v2.0
246 stars 104 forks source link

Produce fixed phabricator statistics #296

Closed kwk closed 1 year ago

kwk commented 1 year ago

This brings much better statistics to the phabricator plugin that can be individually queried with these command line options:

--<section>-differentials-created
--<section>-differentials-closed
--<section>-differentials-accepted
--<section>-differentials-changes-requested
--<section>-differentials-commented

Each section returns ordered results and these results can naturally overlap.

We find the Differentials that might be relevant for the given user and then fetch the transaction events for each Differential to better categorize it and put it in an appropriate bucket (e.g. closed, accepted, etc.). If in the future there's interest in more statistics, this mechanism allows for more categorization.

Differentials are stored and passed around in sets to avoid any duplication. The statistics based upon these sets are converted into sorted lists to have a predictable output.

Verbosity output is now managed with a class variable to avoid storing the verbosity in each Differential.

The tests no longer check for the title of a found Differential but only for it's ID. This way we compensate for a Differential's title that may change over time.

There's now a general mechanism to fetch paged results from phabricator if the endpoint supports it.

Fix: https://github.com/psss/did/issues/288

kwk commented 1 year ago

This is how I ran my tests for the phabricator plugin. It shows nicely how the different tests are being run.

$ export DEBUG=0; export PHABRICATOR_TOKEN=<MY_TOKEN>; DID_DIR=/home/kkleine/dev/did/tmp py.test tests/plugins/test_phabricator.py -v
=============================================================================================== test session starts ===============================================================================================
platform linux -- Python 3.11.1, pytest-7.1.3, pluggy-1.0.0 -- /usr/bin/python3
cachedir: .pytest_cache
rootdir: /home/kkleine/dev/did
collected 11 items                                                                                                                                                                                                

tests/plugins/test_phabricator.py::test_missing_url PASSED                                                                                                                                                  [  9%]
tests/plugins/test_phabricator.py::test_missing_logins PASSED                                                                                                                                               [ 18%]
tests/plugins/test_phabricator.py::test_missing_token PASSED                                                                                                                                                [ 27%]
tests/plugins/test_phabricator.py::test_differentials[--ph-differentials-created-expectations0] PASSED                                                                                                      [ 36%]
tests/plugins/test_phabricator.py::test_differentials[--ph-differentials-closed-expectations1] PASSED                                                                                                       [ 45%]
tests/plugins/test_phabricator.py::test_differentials[--ph-differentials-commented-expectations2] PASSED                                                                                                    [ 54%]
tests/plugins/test_phabricator.py::test_differentials[--ph-differentials-changes-requested-expectations3] PASSED                                                                                            [ 63%]
tests/plugins/test_phabricator.py::test_differentials[--ph-differentials-created --verbose-expectations4] PASSED                                                                                            [ 72%]
tests/plugins/test_phabricator.py::test_differentials[--ph-differentials-closed --verbose-expectations5] PASSED                                                                                             [ 81%]
tests/plugins/test_phabricator.py::test_differentials[--ph-differentials-commented --verbose-expectations6] PASSED                                                                                          [ 90%]
tests/plugins/test_phabricator.py::test_differentials[--ph-differentials-changes-requested --verbose-expectations7] PASSED                                                                                  [100%]

=============================================================================================== 11 passed in 27.15s ===============================================================================================
kwk commented 1 year ago

In case you wonder where the fixes are:

All changes together yielded a few more differentials that otherwise would have been left undetected.

Explanation of the error from before

The error before is so unnoticeable that I have to write it down for my own documentation:

Here's a particular Differential's life-time

*-----------------C------------A----------L
|                 |            |          |
|                 |            |          |
|                 |            |          |
Differential   Commented     Accepted   Landed
Created

My last week at the time of writing was defined like this:

           B------------------------------------E
           |                                    |
        Beginning of                          End of
        last week                             last week

Notice that the B (aka --since) lies right to the differential's create date *.

I am a reviewer for that Differential but I wondered why it didn't show up in my report.

The reason lies in the way that I search for differentials. I had limited the search by the createdStart:

Find revisions created at or after a particular time.

This excluded the particular differential. Once I've started to use modifiedStart instead of createdStart, things turn out fine:

Find revisions modified at or after a particular time.

kwk commented 1 year ago

Is it possible that one stat somehow disappeared?

Thank you @psss for making me aware of this. The problem was not in a missing differential in phabricator but with the way I queried multiple pages from phabricator the cursor was nested in the result dictionary and therefore I didn't fetch enough pages from phabricator because I didn't "see" it. That's fixed now in fbd3faa. Also, I've parallelized the fetching of stats using threading.Thread (Thanks to @fweimer for pointing me at this simple solution).