Skip to content

Commit 75f7a2d

Browse files
per1234per1234
committed
Handle missing deltas data in sketches reports
Previously, the action's code assumed that the sketches reports artifacts would always contain size deltas data. If a report was present that did not contain this data, the action would fail with a `KeyError` error for the missing report key. The most common cause of a compilation not producing size data is a compilation error. In this case the default arduino/compile-sketches workflow configuration would not upload a sketches report workflow artifact (because the upload is done in a subsequent step and GitHub Actions exits the job immediately when a step fails) so the arduino/report-size-deltas action's inability to handle the resulting report format was only an issue in unusual workflow configurations. There is another cause of a compilation not producing size data: the boards platform was not configured to produce the data. Previously, all known Arduino boards platforms produced at least some size data so the problem of the action's lack of compatibility with these boards was only hypothetical. However, the "Arduino Mbed OS Portenta Boards" platform now contains two boards which are missing the configuration to produce the size data: - Portenta H7 - Portenta X8 This meant that the action is broken by any sketches reports produced by compiling for either of those boards. One possible fix would be to make the arduino/compile-sketches action add deltas data objects to the sketches report even when the compilation does not produce any data to calculate data from. But this seems like the wrong approach. So the alternative approach was taken of making the arduino/report-size-deltas action check for the presence of the data. If it is not present, a "N/A" is added to the report in place of the data.
1 parent 7df5255 commit 75f7a2d

File tree

3 files changed

+248
-78
lines changed

3 files changed

+248
-78
lines changed

‎reportsizedeltas/reportsizedeltas.py

Lines changed: 94 additions & 63 deletions
Original file line numberDiff line numberDiff line change
@@ -60,6 +60,7 @@ class ReportSizeDeltas:
6060
token -- GitHub access token
6161
"""
6262
report_key_beginning = "**Memory usage change @ "
63+
not_applicable_indicator = "N/A"
6364

6465
class ReportKeys:
6566
"""Key names used in the sketches report dictionary"""
@@ -344,23 +345,47 @@ def generate_report(self, sketches_reports):
344345
column_heading=size_data[self.ReportKeys.name]
345346
)
346347

347-
# Add the absolute memory data to the cell
348-
summary_report_data[row_number][column_number] = (
349-
get_summary_value(
350-
show_emoji=True,
351-
minimum=size_data[self.ReportKeys.delta][self.ReportKeys.absolute][self.ReportKeys.minimum],
352-
maximum=size_data[self.ReportKeys.delta][self.ReportKeys.absolute][self.ReportKeys.maximum]
348+
# Add the memory data to the cell
349+
if self.ReportKeys.delta in size_data:
350+
# Absolute data
351+
summary_report_data[row_number][column_number] = (
352+
self.get_summary_value(
353+
show_emoji=True,
354+
minimum=size_data[self.ReportKeys.delta][self.ReportKeys.absolute][
355+
self.ReportKeys.minimum],
356+
maximum=size_data[self.ReportKeys.delta][self.ReportKeys.absolute][
357+
self.ReportKeys.maximum]
358+
)
353359
)
354-
)
355360

356-
# Add the relative memory data to the cell
357-
summary_report_data[row_number][column_number + 1] = (
358-
get_summary_value(
359-
show_emoji=False,
360-
minimum=size_data[self.ReportKeys.delta][self.ReportKeys.relative][self.ReportKeys.minimum],
361-
maximum=size_data[self.ReportKeys.delta][self.ReportKeys.relative][self.ReportKeys.maximum]
361+
# Relative data
362+
summary_report_data[row_number][column_number + 1] = (
363+
self.get_summary_value(
364+
show_emoji=False,
365+
minimum=size_data[self.ReportKeys.delta][self.ReportKeys.relative][
366+
self.ReportKeys.minimum],
367+
maximum=size_data[self.ReportKeys.delta][self.ReportKeys.relative][
368+
self.ReportKeys.maximum]
369+
)
370+
)
371+
else:
372+
# Absolute data
373+
summary_report_data[row_number][column_number] = (
374+
self.get_summary_value(
375+
show_emoji=True,
376+
minimum=self.not_applicable_indicator,
377+
maximum=self.not_applicable_indicator
378+
)
379+
)
380+
381+
# Relative data
382+
summary_report_data[row_number][column_number + 1] = (
383+
self.get_summary_value(
384+
show_emoji=False,
385+
minimum=self.not_applicable_indicator,
386+
maximum=self.not_applicable_indicator
387+
)
362388
)
363-
)
364389

365390
# Generate detailed report data
366391
full_report_data = [[fqbn_column_heading]]
@@ -385,15 +410,23 @@ def generate_report(self, sketches_reports):
385410
)
386411
)
387412

388-
# Add the absolute memory data to the cell
389-
full_report_data[row_number][column_number] = (
390-
size_data[self.ReportKeys.delta][self.ReportKeys.absolute]
391-
)
413+
# Add the memory data to the cell
414+
if self.ReportKeys.delta in size_data:
415+
# Absolute
416+
full_report_data[row_number][column_number] = (
417+
size_data[self.ReportKeys.delta][self.ReportKeys.absolute]
418+
)
392419

393-
# Add the relative memory data to the cell
394-
full_report_data[row_number][column_number + 1] = (
395-
size_data[self.ReportKeys.delta][self.ReportKeys.relative]
396-
)
420+
# Relative
421+
full_report_data[row_number][column_number + 1] = (
422+
size_data[self.ReportKeys.delta][self.ReportKeys.relative]
423+
)
424+
else:
425+
# Absolute
426+
full_report_data[row_number][column_number] = self.not_applicable_indicator
427+
428+
# Relative
429+
full_report_data[row_number][column_number + 1] = self.not_applicable_indicator
397430

398431
# Add comment heading
399432
report_markdown = self.report_key_beginning + sketches_reports[0][self.ReportKeys.commit_hash] + "**\n\n"
@@ -427,6 +460,45 @@ def generate_report(self, sketches_reports):
427460
logger.debug("Report:\n" + report_markdown)
428461
return report_markdown
429462

463+
def get_summary_value(self, show_emoji, minimum, maximum):
464+
"""Return the Markdown formatted text for a memory change data cell in the report table.
465+
466+
Keyword arguments:
467+
show_emoji -- whether to add the emoji change indicator
468+
minimum -- minimum amount of change for this memory type
469+
minimum -- maximum amount of change for this memory type
470+
"""
471+
size_decrease_emoji = ":green_heart:"
472+
size_ambiguous_emoji = ":grey_question:"
473+
size_increase_emoji = ":small_red_triangle:"
474+
475+
value = None
476+
if minimum == self.not_applicable_indicator:
477+
value = self.not_applicable_indicator
478+
emoji = None
479+
elif minimum < 0 and maximum <= 0:
480+
emoji = size_decrease_emoji
481+
elif minimum == 0 and maximum == 0:
482+
emoji = None
483+
elif minimum >= 0 and maximum > 0:
484+
emoji = size_increase_emoji
485+
else:
486+
emoji = size_ambiguous_emoji
487+
488+
if value is None:
489+
# Prepend + to positive values to improve readability
490+
if minimum > 0:
491+
minimum = "+" + str(minimum)
492+
if maximum > 0:
493+
maximum = "+" + str(maximum)
494+
495+
value = str(minimum) + " - " + str(maximum)
496+
497+
if show_emoji and (emoji is not None):
498+
value = emoji + " " + value
499+
500+
return value
501+
430502
def comment_report(self, pr_number, report_markdown):
431503
"""Submit the report as a comment on the PR thread
432504
@@ -662,47 +734,6 @@ def get_report_column_number(report, column_heading):
662734
return column_number
663735

664736

665-
def get_summary_value(show_emoji, minimum, maximum):
666-
"""Return the Markdown formatted text for a memory change data cell in the report table.
667-
668-
Keyword arguments:
669-
show_emoji -- whether to add the emoji change indicator
670-
minimum -- minimum amount of change for this memory type
671-
minimum -- maximum amount of change for this memory type
672-
"""
673-
size_decrease_emoji = ":green_heart:"
674-
size_ambiguous_emoji = ":grey_question:"
675-
size_increase_emoji = ":small_red_triangle:"
676-
not_applicable_indicator = "N/A"
677-
678-
value = None
679-
if minimum == not_applicable_indicator:
680-
value = not_applicable_indicator
681-
emoji = None
682-
elif minimum < 0 and maximum <= 0:
683-
emoji = size_decrease_emoji
684-
elif minimum == 0 and maximum == 0:
685-
emoji = None
686-
elif minimum >= 0 and maximum > 0:
687-
emoji = size_increase_emoji
688-
else:
689-
emoji = size_ambiguous_emoji
690-
691-
if value is None:
692-
# Prepend + to positive values to improve readability
693-
if minimum > 0:
694-
minimum = "+" + str(minimum)
695-
if maximum > 0:
696-
maximum = "+" + str(maximum)
697-
698-
value = str(minimum) + " - " + str(maximum)
699-
700-
if show_emoji and (emoji is not None):
701-
value = emoji + " " + value
702-
703-
return value
704-
705-
706737
def generate_markdown_table(row_list):
707738
"""Return the data formatted as a Markdown table
708739

‎reportsizedeltas/tests/data/size-deltas-reports-new/arduino-mbed_portenta-envie_m7.json

Lines changed: 65 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,65 @@
1+
{
2+
"commit_hash": "54815a7d1a30fcb0d77d98242b158e7845c0516d",
3+
"commit_url": "https://example.com/foo",
4+
"boards": [
5+
{
6+
"board": "arduino:mbed_portenta:envie_m7",
7+
"sketches": [
8+
{
9+
"name": "examples/Bar",
10+
"compilation_success": true,
11+
"sizes": [
12+
{
13+
"name": "flash",
14+
"maximum": "N/A",
15+
"current": {
16+
"absolute": "N/A",
17+
"relative": "N/A"
18+
}
19+
},
20+
{
21+
"name": "RAM for global variables",
22+
"maximum": "N/A",
23+
"current": {
24+
"absolute": "N/A",
25+
"relative": "N/A"
26+
}
27+
}
28+
]
29+
},
30+
{
31+
"name": "examples/Foo",
32+
"compilation_success": true,
33+
"sizes": [
34+
{
35+
"name": "flash",
36+
"maximum": "N/A",
37+
"current": {
38+
"absolute": "N/A",
39+
"relative": "N/A"
40+
}
41+
},
42+
{
43+
"name": "RAM for global variables",
44+
"maximum": "N/A",
45+
"current": {
46+
"absolute": "N/A",
47+
"relative": "N/A"
48+
}
49+
}
50+
]
51+
}
52+
],
53+
"sizes": [
54+
{
55+
"name": "flash",
56+
"maximum": "N/A"
57+
},
58+
{
59+
"name": "RAM for global variables",
60+
"maximum": "N/A"
61+
}
62+
]
63+
}
64+
]
65+
}

0 commit comments

Comments
 (0)