lmms
lmms copied to clipboard
Fix memleaks in help/version, including CI tests
Finally ready to review. Thanks to the almost not countable devs who helped.
For discussion:
- Time spent Most systems are equally fast, however, MinGW takes ~ 1 minute more (master average time is 9 minutes), because I added dpkg --add-architecture i386 && apt-get --yes update. I also had to add a second apt-get install, which however just takes ~ 15 seconds. I hope that is all acceptable.
- Kinds of tests Currently, the mentioned PR only runs --help and --version and checks the output. However, I would like to let it export a song to wav, too, since that can be done with the command line. The question is just which song we take. If it is a simple, minimal song, we may have a reproducible output (the WAV is always the same), but we do not test many instruments. Larger songs may cover more instruments, but might have random somewhere, so they might not be deterministic.
@DomClark as a friendly reminder, it seems like you self-requested a review here last month.
There are three things changed in this PR:
- The memory leak fix for the help/version commands. This looks good to me.
- The end-to-end tests for said commands. I still think the build action is the wrong place to define these, especially as separate steps. While end-to-end tests are worth having, I'd rather leave them out for now, and find a proper way to write them. (This could even be done in a separate action, which would enable us to test the MinGW builds in a real Windows environment, rather than under Wine.)
- The introduction of yamllint. This also looks good to me, but is only relevant in the context of the build script changes, so I'd make it a separate PR.
Since this PR has been spun up into #7424 and #7423 , better to close this PR in favour of it's children.
Thanks, but since this contains multiple open comments from Dom, I will rather keep using this for the remaining CI commit. I will fix the title and description.
Oh no now it cannot be reopened :-( OK, I will open a new PR...
- While end-to-end tests are worth having, I'd rather leave them out for now, and find a proper way to write them. (This could even be done in a separate action, which would enable us to test the MinGW builds in a real Windows environment, rather than under Wine.)
Do you mean having 2 actions in sequential order, e.g. let one action use jobs.JOBID.needs?