templates: Break out build/test summaries #320

mrbazzan · 2022-08-22T20:47:57Z

Another approach to #310. It should potentially close #290. Closes #290

Hi @spbnick. Please review when you can.

mrbazzan · 2022-09-12T14:59:38Z

Hi @spbnick, silent reminder to check this PR when you can

spbnick · 2022-09-15T15:14:39Z

Hi @mrbazzan, sorry for the long delay. I have been preparing for and attending the Linux Plumbers conference in the past couple of weeks. Flying home now, and will get back to work next week. I took a brief look at the PR a while ago, but didn't have time to give you a extensive review. One thing I can say already, though, I would prefer to avoid defining extra filters if at all possible. We should be able to process our data in pure Jinja2, if that's too difficult we can change the data we're exposing to make the code simpler, in the worst case we should just make our requirements simpler :)

mrbazzan · 2022-09-16T15:52:02Z

Hi @mrbazzan, sorry for the long delay. I have been preparing for and attending the Linux Plumbers conference in the past couple of weeks. Flying home now, and will get back to work next week.

Hi. No problem. How was the conference? safe journey :)

I took a brief look at the PR a while ago, but didn't have time to give you a extensive review. One thing I can say already, though, I would prefer to avoid defining extra filters if at all possible. We should be able to process our data in pure Jinja2, if that's too difficult we can change the data we're exposing to make the code simpler, in the worst case we should just make our requirements simpler :)

I thought as much; is there any reason for not using extra filters? I'll try to do it with pure Jinja2 again while awaiting an extensive review.

spbnick · 2022-09-26T12:29:30Z

Thank you, @mrbazzan, the travel went smoothly, no problems there, but unfortunately I was among the few who caught COVID at the conference, and am now recovering.

I'd prefer avoiding extra filters because it makes the system much more obscure and complicated. You have to jump between Jinja2 and Python code to figure it out. The worst part is that the Python code is so specific. I'm more-or-less OK with adding some generic filters, like basic string or container operations and such, if we really have to, but not the highly-specific operations. If we want to make the Jinja2 code simpler it might be better to rethink our object representation and the available fields to help it do what it needs.

mrbazzan · 2022-09-27T18:52:51Z

Thank you, @mrbazzan, the travel went smoothly, no problems there, but unfortunately I was among the few who caught COVID at the conference, and am now recovering.

That's sad to hear. Sorry about that. :(
Get well soon!

I'd prefer avoiding extra filters because it makes the system much more obscure and complicated. You have to jump between Jinja2 and Python code to figure it out. The worst part is that the Python code is so specific. I'm more-or-less OK with adding some generic filters, like basic string or container operations and such, if we really have to, but not the highly-specific operations.

I see. This makes sense.

If we want to make the Jinja2 code simpler it might be better to rethink our object representation and the available fields to help it do what it needs.

Since, I'm working on it again (using pure Jinja2) — I'll let you know if I find it necessary. By the way, what do you mean by "rethink our object representation"?

spbnick · 2022-09-28T14:36:44Z

Since, I'm working on it again (using pure Jinja2) — I'll let you know if I find it necessary. By the way, what do you mean by "rethink our object representation"?

Add some fields or change existing fields, make values more uniform, define some fields in base classes, etc.

mrbazzan · 2022-10-10T13:20:05Z

I'm more-or-less OK with adding some generic filters, like basic string or container operations and such, if we really have to, but not the highly-specific operations.

Hi @spbnick,

On generic filters, a filter like that compares each element of two iterables and return the maximum by length.

Does that count as generic?

spbnick · 2022-10-17T11:33:54Z

On generic filters, a filter like that compares each element of two iterables and return the maximum by length.

Does that count as generic?

This is still fairly specific, as we need that only in a couple places (I assume), and it could be implemented in Jinja2.

Something like this could be generic enough to get a filter: https://gitlab.com/cki-project/kpet/-/issues/36

mrbazzan · 2022-10-20T20:01:58Z

Okay. 👍🏿

mrbazzan · 2022-12-09T09:45:33Z

Hello @spbnick,
The failing tests are not from my end. It has something to do with the action setting up Python3.6.

Please review when you can.

mrbazzan · 2022-12-21T09:05:22Z

@spbnick ping.

spbnick

This is looking better, thank you, Abbdulwasiu!

We're getting there. I like how you tucked the overview generation into a macro (there's one inline comment about its placement, BTW).

However, I acutely felt the lack of documentation and comments explaining what's exactly going on, and I have to admit I didn't manage to understand everything in the limited time I had 🙈

Would be great to have some comments explaining what each chunk of such complex code does. OTOH, it wasn't necessary to split writing the overview macro into multiple commits. Just a single commit adding it would be better. Especially since the macro would have no use until it was complete. Adding code comments would've been more useful (although I appreciate trying to split it down and comment in commit messages). And perhaps your code could start the convention of documenting Jinja2 macros in KCIDB 😁

I wonder if we could simplify this further.

Could we perhaps add a macro called e.g. container_emoji_counts to both build.j2 and test.j2, and make each macro take an output dictionary, and a build/test container respectively as the arguments. The macros could then fill the output dictionary with status emojis -> count entries. E.g. the one for a test container could produce:

{
    "❌": 2,
    "✅": 21,
    "🚧✅": 4,
    "🚧❓": 2,
}

The build one would produce the same output, but with a limited set of three emojis. E.g.:

{
    "❌": 20,
    "✅": 60,
    "❓": 2,
}

Then we could have a macro in misc.j2 which would accept an output list and a list of such dictionaries as the arguments, iterate over all possible status emoji strings, highest-priority first (we could hardcode both of those for the start, I suppose), find the longest counts for each, and output the overview strings corresponding to each of them, also substituting the "➖" emoji for padding as needed. E.g. for the above two dictionaries, in order, it would produce:

[
    "❌  2 ✅ 21 ➖   🚧✅ 4 🚧❓ 2",
    "❌ 20 ✅ 60 ❓ 2"
]

Perhaps a simpler interface for that macro could be to simply accept a variable number of arguments (see varargs in Jinja2 docs), each being an emoji count dictionary, and output the overview string for the first one.

So, if we call that macro e.g. emoji_overview, and have the above emoji counts, the Jinja2 code like this:

{{ set build_emoji_counts = {} }}
{% set _ = build_macros.container_emoji_counts(build_emoji_counts, revision) %}
{{ set test_emoji_counts = {} }}
{% set _ = test_macros.container_emoji_counts(test_emoji_counts, revision) %}

    Builds: {{ misc_macros.emoji_overview(build_emoji_counts, test_emoji_counts) }}
     Tests: {{ misc_macros.emoji_overview(test_emoji_counts, build_emoji_counts) }}

Would produce this output:

    Builds: ❌  2 ✅ 21 ➖   🚧✅ 4 🚧❓ 2
     Tests: ❌ 20 ✅ 60 ❓ 2

What do you think?

spbnick · 2023-01-03T12:55:12Z

kcidb/templates/misc.j2

@@ -1,5 +1,7 @@
 {# Miscellaneous macros #}

+{% import "test.j2" as test_macros %}
+


The misc.j2 is more general-purpose than test.j2, so it's best not to include the latter into the former, or include loops may occur.

A better place for macros you're adding could be a separate file, e.g. called overview.j2.

mrbazzan · 2023-01-26T08:58:47Z

Hi @spbnick

Then we could have a macro in misc.j2 which would accept an output list and a list of such dictionaries as the arguments, iterate over all possible status emoji strings, highest-priority first (we could hardcode both of those for the start, I suppose), find the longest counts for each, and output the overview strings corresponding to each of them, also substituting the "heavy_minus_sign" emoji for padding as needed. E.g. for the above two dictionaries, in order, it would produce:

What do you mean by "hardcode"? Do you mean the first three emojis [❌, ✅, ❓]

How do you think about this outputs?

    Builds: ❌  2 ✅ 21 ➖   🚧✅ 4
     Tests: ❌ 20 ➖    ❓ 2

    Builds: ❌  2 ➖  ➖   🚧✅ 4 
     Tests: ❌ 20 ➖ ❓2

In this cases, should I just remove the PASS ( and UNKNOWN) completely since both Build and Test don't record any case?

    Builds: ❌  2 ➖  ➖   🚧✅ 4 
     Tests: ❌ 20 ➖  ❓2

    Builds: ❌  2 ➖ ➖ 🚧✅ 4 
     Tests: ❌ 20 ➖ ➖

spbnick · 2023-01-26T11:20:46Z

Then we could have a macro in misc.j2 which would accept an output list and a list of such dictionaries as the arguments, iterate over all possible status emoji strings, highest-priority first (we could hardcode both of those for the start, I suppose), find the longest counts for each, and output the overview strings corresponding to each of them, also substituting the "heavy_minus_sign" emoji for padding as needed. E.g. for the above two dictionaries, in order, it would produce:

What do you mean by "hardcode"? Do you mean the first three emojis [x, white_check_mark, question]

I meant put the list of emoji strings and their priorities in whatever way necessary for our functions, directly into the template code (beside them or inside them, whatever works). It would duplicate the similar information in kcidb.oo and would introduce the risk of them going out of sync, but it would be good enough for the start.

How do you think about this outputs?

    Builds: ❌  2 ✅ 21 ➖   🚧✅ 4
     Tests: ❌ 20 ➖    ❓ 2

    Builds: ❌  2 ➖  ➖   🚧✅ 4 
     Tests: ❌ 20 ➖ ❓2

Yes, these look pretty good!

The last line looks unaligned (which I suppose is a typo), and we have the unnecessary blank column which we discuss below, but otherwise they look nice, thanks!

In this cases, should I just remove the PASS ( and UNKNOWN) completely since both Build and Test don't record any case?

    Builds: ❌  2 ➖  ➖   🚧✅ 4 
     Tests: ❌ 20 ➖  ❓2

    Builds: ❌  2 ➖ ➖ 🚧✅ 4 
     Tests: ❌ 20 ➖ ➖

Yes, we need to remove the statuses which are not used by either, otherwise listing all the possible statuses would be overwhelming most of the time.

mrbazzan · 2023-02-01T15:50:05Z

Hello @spbnick, ready for review.

The failing tests are not from any files I edited.

mrbazzan · 2023-03-07T09:29:31Z

Ping.

spbnick · 2023-03-07T09:50:50Z

Hi Abdulwasiu! Sorry it's taking so long, by I've been swamped with work the past couple of months, and am still struggling with long COVID symptoms. Now that we're participating in Outreachy there's even less time. Hopefully I'll get to this soon, though. Thank you.

mrbazzan · 2023-03-07T10:01:05Z

Sure. No problem.

mrbazzan · 2023-04-27T07:32:26Z

Hi @spbnick, silent reminder to check this when you can

spbnick · 2023-05-05T15:19:10Z

Thank you, Abdulwasiu. This is quite close to what I was talking about. I went ahead and refined this further in the two commits I put on top. Mostly streamlining the code, and also adding support for handling arbitrary number of emoji dictionaries (what we're using varargs for), not just two. I haven't tested it thoroughly, and it's probably wrong, but this is the direction I think we need to go. Please take a look, ask any questions you have, and take over to finish this.

Thank you, and sorry it took me so long to get around to this.

spbnick · 2023-05-05T10:42:38Z

kcidb/templates/overview.j2

+            {%- if emoji in varargs[1] %}
+                # if "emoji" exists in each of the emoji count dictionaries,
+                # generate a formatting string from the length of the longest
+                # count between them.


Thank you for the comments, they really help!

However, Jinja2 comment syntax is {# ... #}, not # .... Your code works without side-effects only because you assign the result of this macro to a throw-away variable (_). Otherwise they would end in the output.

spbnick · 2023-05-05T15:47:15Z

Oh, and please rebase this on the latest main!

Store waived/status emojis in dictionaries, keyed by the corresponding values, to make it possible to enumerate them. Might be a good idea to expose the similar mappings from the OO module to templates instead, later.

This commit adds a macro that populates a dictionary with the total count of each emoji in WAIVED_STATUS_EMOJIS for a container's build objects.

This commit adds a macro that populates a dictionary with the total count of each emoji in WAIVED_STATUS_EMOJIS for a container's test_roots objects.

This commit adds a public macro called :emoji_counts: which is used to generate overview string for BUILDS or TESTS. This macro simply accepts a variable number of arguments (see https://jinja.palletsprojects.com/en/3.0.x/templates/#macros), each being an emoji count dictionary, and output the overview string for the first one.

mrbazzan · 2023-05-25T10:38:35Z

Mostly streamlining the code, and also adding support for handling arbitrary number of emoji dictionaries (what we're using varargs for), not just two.

Thanks for this. You did the brunt of the job 😄

Thank you, and sorry it took me so long to get around to this.

No problem. Please review when you can

mrbazzan · 2023-06-06T09:40:14Z

@spbnick
Silent ping

spbnick · 2023-07-06T15:48:06Z

Thank you @mrbazzan ❤️, I had some time to look at this this week and most of it looks good, and I'm trying to come up with a test framework to test this. I have only a few comments, which I'll post once I figure it out.

mrbazzan · 2023-07-06T20:12:13Z

Thank you @mrbazzan ❤️, I had some time to look at this this week

Thanks for taking the time. 👍🏿

I'm trying to come up with a test framework to test this. I have only a few comments, which I'll post once I figure it out.

Sure. I'll be waiting for the comments.

NB: I've been super free these days, if there is any urgent (or not) issues you need work done on. I'll be glad to help

mrbazzan mentioned this pull request Aug 22, 2022

templates: Break out build/test summaries #310

Draft

mrbazzan force-pushed the notif branch from 4ab2926 to d4be653 Compare August 22, 2022 21:48

mrbazzan force-pushed the notif branch from d4be653 to 3b5456d Compare December 9, 2022 09:43

mrbazzan mentioned this pull request Dec 9, 2022

Consider breaking out build/test summaries #290

Open

spbnick requested changes Jan 4, 2023

View reviewed changes

mrbazzan force-pushed the notif branch from 3b5456d to ec731d9 Compare February 1, 2023 15:02

spbnick requested changes May 5, 2023

View reviewed changes

spbnick had a problem deploying to staging May 5, 2023 15:24 — with GitHub Actions Failure

spbnick and others added 2 commits May 25, 2023 08:53

templates: Store emojis in dictionaries

e681f0f

Store waived/status emojis in dictionaries, keyed by the corresponding values, to make it possible to enumerate them. Might be a good idea to expose the similar mappings from the OO module to templates instead, later.

templates: Add macro to get all build emoji

2df37da

This commit adds a macro that populates a dictionary with the total count of each emoji in WAIVED_STATUS_EMOJIS for a container's build objects.

mrbazzan added 4 commits May 25, 2023 09:35

templates: Add macro to get all test emoji

626f801

This commit adds a macro that populates a dictionary with the total count of each emoji in WAIVED_STATUS_EMOJIS for a container's test_roots objects.

templates: Change overview for checkout description

0a3dee0

templates: Change overview for revision description

b3c37e7

mrbazzan force-pushed the notif branch from a22d75f to b3c37e7 Compare May 25, 2023 10:05

mrbazzan had a problem deploying to staging May 25, 2023 10:15 — with GitHub Actions Failure

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

templates: Break out build/test summaries #320

templates: Break out build/test summaries #320

mrbazzan commented Aug 22, 2022 •

edited

Loading

mrbazzan commented Sep 12, 2022

spbnick commented Sep 15, 2022

mrbazzan commented Sep 16, 2022

spbnick commented Sep 26, 2022

mrbazzan commented Sep 27, 2022

spbnick commented Sep 28, 2022

mrbazzan commented Oct 10, 2022

spbnick commented Oct 17, 2022

mrbazzan commented Oct 20, 2022

mrbazzan commented Dec 9, 2022

mrbazzan commented Dec 21, 2022

spbnick left a comment

spbnick Jan 3, 2023

mrbazzan commented Jan 26, 2023 •

edited

Loading

spbnick commented Jan 26, 2023

mrbazzan commented Feb 1, 2023

mrbazzan commented Mar 7, 2023

spbnick commented Mar 7, 2023

mrbazzan commented Mar 7, 2023

mrbazzan commented Apr 27, 2023

spbnick commented May 5, 2023

spbnick May 5, 2023

spbnick commented May 5, 2023

mrbazzan commented May 25, 2023

mrbazzan commented Jun 6, 2023

spbnick commented Jul 6, 2023

mrbazzan commented Jul 6, 2023

		@@ -1,5 +1,7 @@
		{# Miscellaneous macros #}

		{% import "test.j2" as test_macros %}

templates: Break out build/test summaries #320

Are you sure you want to change the base?

templates: Break out build/test summaries #320

Conversation

mrbazzan commented Aug 22, 2022 • edited Loading

mrbazzan commented Sep 12, 2022

spbnick commented Sep 15, 2022

mrbazzan commented Sep 16, 2022

spbnick commented Sep 26, 2022

mrbazzan commented Sep 27, 2022

spbnick commented Sep 28, 2022

mrbazzan commented Oct 10, 2022

spbnick commented Oct 17, 2022

mrbazzan commented Oct 20, 2022

mrbazzan commented Dec 9, 2022

mrbazzan commented Dec 21, 2022

spbnick left a comment

Choose a reason for hiding this comment

spbnick Jan 3, 2023

Choose a reason for hiding this comment

mrbazzan commented Jan 26, 2023 • edited Loading

spbnick commented Jan 26, 2023

mrbazzan commented Feb 1, 2023

mrbazzan commented Mar 7, 2023

spbnick commented Mar 7, 2023

mrbazzan commented Mar 7, 2023

mrbazzan commented Apr 27, 2023

spbnick commented May 5, 2023

spbnick May 5, 2023

Choose a reason for hiding this comment

spbnick commented May 5, 2023

mrbazzan commented May 25, 2023

mrbazzan commented Jun 6, 2023

spbnick commented Jul 6, 2023

mrbazzan commented Jul 6, 2023

mrbazzan commented Aug 22, 2022 •

edited

Loading

mrbazzan commented Jan 26, 2023 •

edited

Loading