Add tests verifying PERFETTO value

[sudara]

Resolves #9

[sudara]

Looking good!

The python file seems to have that same whitespace issue we were seeing on other PRs and windows looks like it's failing a test....

[benthevining]

What whitespace issue? Should I be using spaces instead of tabs?

[sudara]

@benthevining Since the repo was with spaces, maybe makes sense to be consistent vs. mix them, but I was referring to this. Haven't checked out the branch locally to see if it's just a github rendering issue or not:

[benthevining]

That's a Github rendering issue, since that file currently uses tabs. I'll convert it to spaces.

[benthevining]

The Windows test failure is due to this build error:

Microsoft (R) Build Engine version 17.0.0+c9eb9dd64 for .NET Framework
Copyright (C) Microsoft Corporation. All rights reserved.

  Checking File Globs
  perfetto.vcxproj -> C:\Users\benth\Documents\melatonin_perfetto\Builds\Debug\perfetto.lib
  melatonin_perfetto.cpp
C:\Users\benth\Documents\melatonin_perfetto\melatonin_perfetto\melatonin_perfetto.cpp(4,1): error C2766: explicit speci
alization; 'static_state_' has already been defined [C:\Users\benth\Documents\melatonin_perfetto\Builds\tests\PerfettoV
alue\PerfettoValueTest.vcxproj]
C:\Users\benth\Documents\melatonin_perfetto\melatonin_perfetto\melatonin_perfetto.h(30): message : see previous definit
ion of 'private: static perfetto::internal::DataSourceStaticState perfetto::DataSource<perfetto::TrackEvent,perfetto::i
nternal::TrackEventDataSourceTraits>::static_state_' [C:\Users\benth\Documents\melatonin_perfetto\Builds\tests\Perfetto
Value\PerfettoValueTest.vcxproj]
  juce_core.cpp
  Generating Code...

Do you have any ideas about this?

[sudara]

Dang, thought I wrote you back yesterday, sorry about that. I actually don't have any idea on that, haven't seen it before. Is it possible melatonin_perfetto is being included twice on a test somehow?

[benthevining]

Hmm... I'm not sure. It's curious that this issue only happens on Windows and not Mac.

[benthevining]

I can re-examine all the CMake to check for something like that, but otherwise, I'm tempted to report this as a Perfetto bug...

[sudara]

The issue only bubbles up in the one particular test melatonin.perfetto.PERFETTO_symbol_value? It looks like those cpp files also include melatonin_perfetto.h, maybe that's not necessary?

[benthevining]

I suspect this issue would happen in any project that consumes melatonin_perfetto, but I haven't tested that theory yet.

That cpp file needs to include the module header to get the default PERFETTO symbol definition, that's the whole idea of that test

[sudara]

Ah, perhaps the problem is this macro is needed in a cpp file: https://github.com/sudara/melatonin_perfetto/blob/534389e91c5879ff5c98e0498d5c8434ed2f3466/melatonin_perfetto/melatonin_perfetto.cpp#L4

[benthevining]

Right, but the test app links to the module, so it gets that cpp file. The error I get is:

C:\Users\benth\Documents\melatonin_perfetto\melatonin_perfetto\melatonin_perfetto.cpp(4,1): error C2766: explicit speci
alization; 'static_state_' has already been defined

I've reviewed all the CMake, and nothing is being linked or added twice, everything in the module set up looks correct to me. I wonder if any other Perfetto users have ever reported something similar...

[benthevining]

I've filed this issue at the Perfetto repo to see if anyone has ever seen a similar error.

[sudara]

See: https://github.com/google/perfetto/issues/214

[benthevining]

Ah, brilliant, great find! I can add the /permissive- flag as a workaround until this is fixed in MSVC. (Looks like they reported it fixed, but people are still reporting errors.)

I'll test it out now, and commit if it works...

[benthevining]

The build and tests succeed on my Windows machine with the /permissive- flag added, so I've pushed that fix, let's hope the CI succeeds...

[sudara]

Awesome, thanks for putting comments in there. Going to test failure...

[benthevining]

Both Mac and Windows tests failed after that commit, which is the behavior we expect. I've now reverted that commit (so the default value of PERFETTO is now 0 again).

The last thing left on my to-do list is to test that dump files are created, but other than that, I think this PR is ready to be merged.

[sudara]

Thanks for that! Wasn't sure if you wanted to merge this or add the file test, I'll let you wrap that up!

[benthevining]

I'll merge this PR now and open a new one for the dump file creation test.