You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
to both test the examples against expected output and to collect size info to track regressions (size info needs ocular inspection and there is no automatic whistle for regressions).
Potential solution:
Separate size info collection from functional test into its own set of tests.
Automated regression tests against previous (released) version of the framework.
Integration of regression test in CI workflow.
Advantages:
In order to catch errors in the framework, Results are unwrapped(). This brings in a lot of bloat (panic formatting). Hence size info reflects not the framework in particular but rather the whole application. This makes it very hard to isolate minor bloat caused by the changes in the framework, to the major bloat introduced by the formatting. The size info tests should ok() instead of unwrap() the Results thus no panic formatting bloat would be introduced, resulting in size info representative to the internals of RTIC.
Automation and Integration to CI would reduce the risk for bloating bugs to slip through. Moreover since test are run under the latest toolchain, it would (to some extent) work as a test for updates on the Rust toolchain as well.
Size info collection does not need to execute the examples, just compile and retrieve size info (much less time consuming). This is useful to reduce iteration time during development and bug searching (and complements the functional test).
Challenges:
Exactly how size info should be compared.
Should old size info be stored as part of the repository or should it be generated on the fly (by compiling an old version of the repository).
Should this be tag based, by Crates.io, or some other tagging mechanism.
Should a regression be just informative or generate a hard fail (actually preventing regressions from becoming published)
Should tool-chain regressions be separated from RTIC regressions. (This has an impact to storing vs re-creating size info.)
Should some slack in regressions be allowed. Many of the optimizations rely on LLVM being able to see through deeply nested closures etc. To this end LLVM relies on heuristics tuned by various parameters (thus, a few bytes win or loose may not indicate a true improvement or regression.
Feedback and Help wanted:
If you want to contribute to the development of automated size regression testing feel free to comment on this issue and we can take it from there.
/Per
The text was updated successfully, but these errors were encountered:
Problem:
Currently we run
to both test the examples against expected output and to collect size info to track regressions (size info needs ocular inspection and there is no automatic whistle for regressions).
Potential solution:
Advantages:
In order to catch errors in the framework,
Results
areunwrapped()
. This brings in a lot of bloat (panic
formatting). Hence size info reflects not the framework in particular but rather the whole application. This makes it very hard to isolate minor bloat caused by the changes in the framework, to the major bloat introduced by the formatting. The size info tests shouldok()
instead ofunwrap()
theResults
thus nopanic
formatting bloat would be introduced, resulting in size info representative to the internals of RTIC.Automation and Integration to CI would reduce the risk for bloating bugs to slip through. Moreover since test are run under the latest toolchain, it would (to some extent) work as a test for updates on the Rust toolchain as well.
Size info collection does not need to execute the examples, just compile and retrieve size info (much less time consuming). This is useful to reduce iteration time during development and bug searching (and complements the functional test).
Challenges:
Feedback and Help wanted:
If you want to contribute to the development of automated size regression testing feel free to comment on this issue and we can take it from there.
/Per
The text was updated successfully, but these errors were encountered: