sudara / pamplejuce

A JUCE audio plugin template. JUCE 7, Catch2, Pluginval, macOS notarization, Azure Trusted Signing, Github Actions
https://melatonin.dev/blog/
MIT License
406 stars 39 forks source link

Request: add a `MessageManager` fixture for testing plugins that need it. #18

Closed JoseDiazRohena closed 10 months ago

JoseDiazRohena commented 1 year ago

AudioProcessor implementations may have a dependency on juce::MessageManager. As far as I can tell, the message manager is instantiated by the plugin editor, which we don't create (or want to create) when running tests. As such, testing a plugin that needs a message manager for the processor, such as when juce::AudioParameterValueTreeState is used to manage parameters, results in an assert.

This issue can be trivially resolved by getting a pointer to the message manager via it's static accessor before APVTS needs it—that's not every nice.

It would be great to have an fixture that, when added to the processor (probably behind some kind of #if TEST macro), will ensure that this message manager is properly instantiated and de instantiated

sudara commented 1 year ago

I finally took a deeper look here.

I do various different things to get around this issue in tests:

Use a loose ScopedJuceInitialiser_GUI

This is probably easiest and most agile.

If I just want to do something that requires the message thread running (for example test against the UndoManager or have changeListeners fire), I'll add this code in the test:

auto gui = juce::ScopedJuceInitialiser_GUI {};

Use a MockProcessor

When I need to interact with the apvts in tests, I use a very stripped down mock AudioProcessor called MockProcessor.h/cpp.

This constructs an apvts and has a member juce::ScopedJuceInitialiser_GUI gui; (to start up the UI when the processor starts).

This has some additional benefit of encouraging your own processor to be as basic as possible (vs. stuffed with tons of app-specific stuff).

The version I use also has boots up some app-specific logic, so not totally sure how useful it is to stuff it in pamplejuce.

Wrapper around the whole UI

I also have a little helper that wraps the whole stack, "integration test" style... this is pretty cumbersome but needed in some cases, for example benchmarking startup time or doing screenshots.

In this case, again, just having a ScopedJuceInitialiser_GUI running alongside your actual plugin is enough to make things happy.

void runWithinPluginEditor (const std::function<void (YourPluginAudioProcessor& plugin)>& testCode)
{
    YourPluginAudioProcessor plugin;
    auto gui = juce::ScopedJuceInitialiser_GUI {};
    auto editor = plugin.createEditorIfNeeded();

    testCode (plugin);

    plugin.editorBeingDeleted (editor);
    delete editor;
}
runWithinPluginEditor ([&] (YourPluginAudioProcessor& plugin) {
    // your code
    REQUIRE (...);
});
JoseDiazRohena commented 1 year ago

Ah. I didn't Juce had something for this! Honestly, I think Pamplejuce should at least use this juce initialiser you are talking about. Otherwise, the template doesn't really work for a lot of real projects.

sudara commented 1 year ago

Otherwise, the template doesn't really work for a lot of real projects.

Not really sure what you mean by this. This use case isn't relevant for like 95% of my Catch2 tests, and I assume others as well. It's only if you are testing some quite intensive things with apvts or async callbacks (which Pamplejuce remains agnostic about) that it becomes relevant. And ultimately, the difficulty is because JUCE didn't design these things with testability in mind.

I'm not totally sure what example would be appropriate for Pamplejuce (whose goal is to get you up and running with CMake, Catch, CI), but feel free to submit a PR if you have a good example test!

JoseDiazRohena commented 1 year ago

https://github.com/sudara/pamplejuce/blob/83891071a1f502e223ebbb5e5373017ce614e227/Tests/PluginBasics.cpp#L11

Instantiating a plugin processor here causes an assert if that processor uses APVTS. So I'm thinking that, if what's being shown in the template is that you can instantiate your Audio Processor and test it, that it should work when what I think is a common way of managing parameters for an Audio Processor.

A perfect use of this template, then, will not build if the audio processor uses APVTS. I think that sends the wrong message when the user is already absorbing a lot of new information.

This, however, is a problem that will probably only really bother people migrating older projects to Cmake. Also, it's not reasonable to anticipate every case—many projects don't use it. I guess I just mean that a cool extension of the tools on offer here would be something that helps you instantiate an audio processor for tests as shown in the template if your audio processor uses APVTS.

I'm also biased because this is very tied to my experience of using the template.

sudara commented 1 year ago

So as far as a pragmatic solution to resolve the issue (given that Pamplejuce doesn't use the apvts) what would you recommend? What would have helped you? A commented out line like this in the tests?

// uncomment if using the apvts, as the message manager needs to be running
// see more: https://github.com/sudara/pamplejuce/issues/18
// auto gui = juce::ScopedJuceInitialiser_GUI {};