Handle sketches reports with missing size data #31
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
In addition to the per-sketch memory usage data, the sketches report includes per-board data that summarizes the
per-sketch data.
Previous versions of the arduino/compile-sketches action did not add the key for the summary data to the report when
none of the compilations produced size data. When the arduino/report-size-deltas action attempted to parse a sketches
report with this format, it would error:
```
Traceback (most recent call last):
File "/reportsizedeltas/reportsizedeltas.py", line 737, in <module>
main() # pragma: no cover
File "/reportsizedeltas/reportsizedeltas.py", line 32, in main
report_size_deltas.report_size_deltas()
File "/reportsizedeltas/reportsizedeltas.py", line 95, in report_size_deltas
self.report_size_deltas_from_workflow_artifacts()
File "/reportsizedeltas/reportsizedeltas.py", line 149, in report_size_deltas_from_workflow_artifacts
sketches_reports = self.get_sketches_reports(artifact_folder_object=artifact_folder_object)
File "/reportsizedeltas/reportsizedeltas.py", line 295, in get_sketches_reports
not in report_data[self.ReportKeys.boards][0][self.ReportKeys.sizes][0])
KeyError: 'sizes'
```
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.
The arduino/compile-sketches action was changed to always add a `boards[*].sizes` object to the sketches report, even
when it doesn't have any size data to fill it. So the problem will no longer occur with sketches reports produced by the
new version of that action. However, repositories may still contain sketches report artifacts with the old format so the
arduino/report-size-deltas action must be able to continue to work in the presence of these reports. That is
accomplished by skipping any report that is missing a `boards[*].sizes` key.
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.
Flake8 is configured to enforce a cyclomatic complexity limit of 10 on the project. The recent changes caused an increase of the cyclomatic complexity in ReportSizeDeltas.generate_report to 12. In order to reduce this, code was moved into dedicated functions.
per1234
added
type: imperfection
Codecov ReportPatch coverage:
Additional details and impacted files@@ Coverage Diff @@
## main #31 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 2 2
Lines 707 726 +19
=========================================
+ Hits 707 726 +19
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. ☔ View full report in Codecov by Sentry. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
KeyErrorerror for keys missing from the report.For example:
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:
upload.maximum_sizeproperty for Portenta H7 ArduinoCore-mbed#642)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 all expected objects to the sketches report even when the compilation does not produce any data to populated them with. 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.