Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

merge 2.3 #4588

Merged
merged 5 commits into from
Dec 28, 2021
Merged

merge 2.3 #4588

merged 5 commits into from
Dec 28, 2021

Conversation

ronso0
Copy link
Member

@ronso0 ronso0 commented Dec 28, 2021

tests were failing locally so I open this PR.

	157 - ControlObjectScriptTest.CompressingProxyCompareCount1 (Failed)
	158 - ControlObjectScriptTest.CompressingProxyCompareValue1 (Failed)
	159 - ControlObjectScriptTest.CompressingProxyCompareCount2 (Failed)
	160 - ControlObjectScriptTest.CompressingProxyCompareValue2 (Failed)
	161 - ControlObjectScriptTest.QueuedCompareCount2 (Failed)
	162 - ControlObjectScriptTest.QueuedCompareValue2 (Failed)
	163 - ControlObjectScriptTest.CompressingProxyCompareCountMulti (Failed)
	164 - ControlObjectScriptTest.CompressingProxyCompareValueMulti (Failed)
	165 - ControlObjectScriptTest.CompressingProxyMultiConnection (Failed)
	166 - ControlObjectScriptTest.QueuedFallbackMultiConnection (Failed)
	167 - ControlObjectScriptTest.CompressingProxyManyEvents (Failed)

Any ideas why @JoergAtGithub ?

daschuer and others added 5 commits December 26, 2021 23:32
* Add missing entries to CHANGELOG.md
* Moved some changelog entries to the packaging section
* Remove changelog entries that are not relevant for a user
DlgTrackInfo: ensure Beats pointer before scaling beats
Copy link
Contributor

@uklotzde uklotzde left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

CI succeeded, LGTM

Nevertheless, we need to find out the reason for the local test failures. I didn't experience any issues.

@uklotzde uklotzde merged commit e3062a1 into mixxxdj:main Dec 28, 2021
@ronso0
Copy link
Member Author

ronso0 commented Dec 28, 2021

comment race condition :)

@JoergAtGithub
Copy link
Member

None of the changed files is related to this test. Can it be, that just the testcase was rebuilt/pulled from upstream, but not the Mixxx code itself? Was src/control/controlobjectscript.cpp up to date in your local working copy?

@ronso0
Copy link
Member Author

ronso0 commented Dec 28, 2021

Of course merging 2.3 doesn't touch CompressingProxy. I built with feshly pulled main.
I just cleaned my build dir and reconfigured, will see if that fixes it.

@ronso0
Copy link
Member Author

ronso0 commented Dec 29, 2021

Hmm, it's still only those test that fail

	157 - ControlObjectScriptTest.CompressingProxyCompareCount1 (Failed)
	158 - ControlObjectScriptTest.CompressingProxyCompareValue1 (Failed)
	159 - ControlObjectScriptTest.CompressingProxyCompareCount2 (Failed)
	160 - ControlObjectScriptTest.CompressingProxyCompareValue2 (Failed)
	161 - ControlObjectScriptTest.QueuedCompareCount2 (Failed)
	162 - ControlObjectScriptTest.QueuedCompareValue2 (Failed)
	163 - ControlObjectScriptTest.CompressingProxyCompareCountMulti (Failed)
	164 - ControlObjectScriptTest.CompressingProxyCompareValueMulti (Failed)
	165 - ControlObjectScriptTest.CompressingProxyMultiConnection (Failed)
	166 - ControlObjectScriptTest.QueuedFallbackMultiConnection (Failed)
	167 - ControlObjectScriptTest.CompressingProxyManyEvents (Failed)

details:
CompressProxyTests_Fail.txt

@JoergAtGithub
Copy link
Member

The details show the correct warning messages from the CompressingProxy in the error cases. So the proxy itself is operating.
But something with Google-Mock seems to fail. Mocked is only the function slotValueChanged, and arguments are used to distinguish the expected calls of it.

I wonder about this message:
GMOCK WARNING:
/home/ronso/Downloads/mixxx_source/src/master/src/test/controlobjectscripttest.cpp:100: Too many actions specified in EXPECT_CALL(*coScript2, slotValueChanged(_, _))...

Do you use the same Googletest/Googlmock than on CI?

@ronso0
Copy link
Member Author

ronso0 commented Dec 29, 2021

Do you use the same Googletest/Googlmock than on CI?

How do I find out?
Anyway, I just re-ran tools/debian_buildenv.sh and got lcov installed.. 🤷
Now the tests pass 👍

Should cmake require lcov when building tests to avoid such errors?

@JoergAtGithub
Copy link
Member

I found out, that gtest and gmock are always the same, because they are taken from the lib directory in ./mixxx.
I can only guess, that the execution of tools/debian_buildenv.sh fixed something else in your environment.

@JoergAtGithub
Copy link
Member

@ronso0 I looked on the posted test output again, this time on my laptop screen, last time I read it on my phone. Now I see, that multiple tests seem to run overlapped in parallel, while on CI, they run stricly serial.

Your local output (Start of ManyEvents followed by result of CompareCountMulti):
Start 167: ControlObjectScriptTest.CompressingProxyManyEvents 161/729 Test #163: ControlObjectScriptTest.CompressingProxyCompareCountMulti................................***Failed 0.21 sec
CI output:
Start 167: ControlObjectScriptTest.CompressingProxyManyEvents 167/729 Test #167: ControlObjectScriptTest.CompressingProxyManyEvents ...................................... Passed 0.06 sec

Could you try to execute only a single test, intead of all. Does it pass than?

@ronso0
Copy link
Member Author

ronso0 commented Dec 30, 2021

Hmm fails, too:

$ ctest -R ControlObjectScriptTest.CompressingProxyCompareCount1 --output-on-failure

Test project /home/ronso/Downloads/mixxx_source/src/master_build/Debug
    Start 157: ControlObjectScriptTest.CompressingProxyCompareCount1
1/1 Test #157: ControlObjectScriptTest.CompressingProxyCompareCount1 ...***Failed    0.14 sec
QML debugging is enabled. Only use this in a safe environment.
Note: Google Test filter = ControlObjectScriptTest.CompressingProxyCompareCount1
[==========] Running 1 test from 1 test suite.
[----------] Global test environment set-up.
[----------] 1 test from ControlObjectScriptTest
[ RUN      ] ControlObjectScriptTest.CompressingProxyCompareCount1

GMOCK WARNING:
/src/test/controlobjectscripttest.cpp:100: Too many actions specified in EXPECT_CALL(*coScript2, slotValueChanged(_, _))...
Expected to be never called, but has 1 WillOnce()./src/master/src/test/controlobjectscripttest.cpp:104: Failure
Actual function call count doesn't match EXPECT_CALL(*coScript1, slotValueChanged(_, _))...
         Expected: to be called once
           Actual: never called - unsatisfied and active
[  FAILED  ] ControlObjectScriptTest.CompressingProxyCompareCount1 (0 ms)
[----------] 1 test from ControlObjectScriptTest (0 ms total)

[----------] Global test environment tear-down
[==========] 1 test from 1 test suite ran. (0 ms total)
[  PASSED  ] 0 tests.
[  FAILED  ] 1 test, listed below:
[  FAILED  ] ControlObjectScriptTest.CompressingProxyCompareCount1

 1 FAILED TEST


0% tests passed, 1 tests failed out of 1

Total Test time (real) =   0.17 sec

The following tests FAILED:
	157 - ControlObjectScriptTest.CompressingProxyCompareCount1 (Failed)
Errors while running CTest

@JoergAtGithub
Copy link
Member

Here the local output for the same test looks as follows:

C:\Users\Joerg\source\repos\JoergAtGithub\mixxx\build\x64__portable>ctest -R ControlObjectScriptTest.CompressingProxyCompareCount1 --verbose
UpdateCTestConfiguration  from :C:/Users/Joerg/source/repos/JoergAtGithub/mixxx/build/x64__portable/DartConfiguration.tcl
Parse Config file:C:/Users/Joerg/source/repos/JoergAtGithub/mixxx/build/x64__portable/DartConfiguration.tcl
UpdateCTestConfiguration  from :C:/Users/Joerg/source/repos/JoergAtGithub/mixxx/build/x64__portable/DartConfiguration.tcl
Parse Config file:C:/Users/Joerg/source/repos/JoergAtGithub/mixxx/build/x64__portable/DartConfiguration.tcl
Test project C:/Users/Joerg/source/repos/JoergAtGithub/mixxx/build/x64__portable
Constructing a list of tests
Done constructing a list of tests
Updating test list for fixtures
Added 0 tests to meet fixture requirements
Checking test dependency graph...
Checking test dependency graph end
test 157
    Start 157: ControlObjectScriptTest.CompressingProxyCompareCount1

157: Test command: C:\Users\Joerg\source\repos\JoergAtGithub\mixxx\build\x64__portable\mixxx-test.exe "--gtest_filter=ControlObjectScriptTest.CompressingProxyCompareCount1" "--logLevel" "info"
157: Test timeout computed to be: 1500
157: Note: Google Test filter = ControlObjectScriptTest.CompressingProxyCompareCount1
157: [==========] Running 1 test from 1 test suite.
157: [----------] Global test environment set-up.
157: [----------] 1 test from ControlObjectScriptTest
157: [ RUN      ] ControlObjectScriptTest.CompressingProxyCompareCount1
157:
157: GMOCK WARNING:
157: ..\..\src\test\controlobjectscripttest.cpp(100): Too many actions specified in EXPECT_CALL(*coScript2, slotValueChanged(_, _))...
157: Expected to be never called, but has 1 WillOnce().[       OK ] ControlObjectScriptTest.CompressingProxyCompareCount1 (29 ms)
157: [----------] 1 test from ControlObjectScriptTest (29 ms total)
157:
157: [----------] Global test environment tear-down
157: [==========] 1 test from 1 test suite ran. (30 ms total)
157: [  PASSED  ] 1 test.
1/1 Test #157: ControlObjectScriptTest.CompressingProxyCompareCount1 ...   Passed    2.78 sec

The following tests passed:
        ControlObjectScriptTest.CompressingProxyCompareCount1

100% tests passed, 0 tests failed out of 1

Total Test time (real) =   2.93 sec

C:\Users\Joerg\source\repos\JoergAtGithub\mixxx\build\x64__portable>

@ronso0
Copy link
Member Author

ronso0 commented Dec 31, 2021

so the diff starts with the third line

mine

157: GMOCK WARNING:
157: /../../src/test/controlobjectscripttest.cpp:100: Too many actions specified in EXPECT_CALL(*coScript2, slotValueChanged(_, _))...
157: Expected to be never called, but has 1 WillOnce()./../../src/master/src/test/controlobjectscripttest.cpp:104: Failure
157: Actual function call count doesn't match EXPECT_CALL(*coScript1, slotValueChanged(_, _))...
157:          Expected: to be called once
157:            Actual: never called - unsatisfied and active
157: [  FAILED  ] ControlObjectScriptTest.CompressingProxyCompareCount1 (4 ms)

yours

157: GMOCK WARNING:
157: ..\..\src\test\controlobjectscripttest.cpp(100): Too many actions specified in EXPECT_CALL(*coScript2, slotValueChanged(_, _))...
157: Expected to be never called, but has 1 WillOnce().[       OK ] ControlObjectScriptTest.CompressingProxyCompareCount1 (29 ms)
157: [----------] 1 test from ControlObjectScriptTest (29 ms total)

@JoergAtGithub
Copy link
Member

JoergAtGithub commented Dec 31, 2021

I guess this WillOnce is contradictory, because before it's specified, that it will occur zero times.
grafik
But I still don't understand, why Google-Mock report this as error for you, but not elsewhere.
Could you try to remove line 102.

@ronso0
Copy link
Member Author

ronso0 commented Dec 31, 2021

with line 102 commented out it fails at line 104 "expected once but never called"

@JoergAtGithub
Copy link
Member

Is the warning

GMOCK WARNING:
/src/test/controlobjectscripttest.cpp:100: Too many actions specified in EXPECT_CALL(*coScript2, slotValueChanged(_, _))...

gone?

@JoergAtGithub
Copy link
Member

In your output there is the additional line for each test case:
QML debugging is enabled. Only use this in a safe environment.
Can you try to disable this?
BTW: The #include <QtDebug> is not needed, but I could find a relationship tothis message.

@JoergAtGithub
Copy link
Member

I found that controllerscriptenginelegacy_test.cpp, which also uses application()->processEvents(); calls this function always twice to get all events. Maybe this helps also here.

@ronso0
Copy link
Member Author

ronso0 commented Jan 1, 2022

Is the warning

GMOCK WARNING:
/src/test/controlobjectscripttest.cpp:100: Too many actions specified in EXPECT_CALL(*coScript2, slotValueChanged(_, _))...

gone?

Yes. The output is now

157: Test command: /../../src/master_build/Debug/mixxx-test "--gtest_filter=ControlObjectScriptTest.CompressingProxyCompareCount1" "--logLevel" "info"
157: Environment variables: 
157:  QT_QPA_PLATFORM=offscreen
157: Test timeout computed to be: 1500
157: QML debugging is enabled. Only use this in a safe environment.
157: Note: Google Test filter = ControlObjectScriptTest.CompressingProxyCompareCount1
157: [==========] Running 1 test from 1 test suite.
157: [----------] Global test environment set-up.
157: [----------] 1 test from ControlObjectScriptTest
157: [ RUN      ] ControlObjectScriptTest.CompressingProxyCompareCount1
157: /../../src/master/src/test/controlobjectscripttest.cpp:104: Failure
157: Actual function call count doesn't match EXPECT_CALL(*coScript1, slotValueChanged(_, _))...
157:          Expected: to be called once
157:            Actual: never called - unsatisfied and active
157: [  FAILED  ] ControlObjectScriptTest.CompressingProxyCompareCount1 (0 ms)
157: [----------] 1 test from ControlObjectScriptTest (0 ms total)
157: 
157: [----------] Global test environment tear-down
157: [==========] 1 test from 1 test suite ran. (0 ms total)
157: [  PASSED  ] 0 tests.
157: [  FAILED  ] 1 test, listed below:
157: [  FAILED  ] ControlObjectScriptTest.CompressingProxyCompareCount1
157: 
157:  1 FAILED TEST
1/1 Test #157: ControlObjectScriptTest.CompressingProxyCompareCount1 ...***Failed    0.13 sec

I don't know how to disable QML debugging. Remove the QDebug include doesn't help, and I have no idea where mixxx-test arguments are documented.

@JoergAtGithub
Copy link
Member

Could you please try to double line 108 application()->processEvents(); and execute the test again.

@ronso0
Copy link
Member Author

ronso0 commented Jan 1, 2022

👍 then it'll pass, even if I re-enable line 102

@JoergAtGithub
Copy link
Member

Great! Could you try this for the other tests as well.
They are much more complicated cases, I wonder, if duplication of application()->processEvents(); helps everywhere.

@ronso0
Copy link
Member Author

ronso0 commented Jan 2, 2022

Jup, simply duplicating every application()->processEvents(); line makes all tests pass.

@JoergAtGithub
Copy link
Member

That's great to hear, I will prepare a PR tomorrow!

@JoergAtGithub
Copy link
Member

I guess this is, because the QML debugger is hooked in the event pipeline, and it needs one process step from the testcase to the QML debugger, and a second from the debugger to my compressing proxy.

@ronso0
Copy link
Member Author

ronso0 commented Jan 2, 2022

Still, why would it fail on my machine in the first place?
Looking at the Github actions Ubuntu 20.04 build I don't spot a difference except I do not use --config RelWithDebInfo when building. Apart from that everything seems to be identical.

@JoergAtGithub
Copy link
Member

That fits into the picture:

mixxx/CMakeLists.txt

Lines 1086 to 1090 in 6e3295f

# QML Debugging
if(CMAKE_BUILD_TYPE STREQUAL "Debug")
target_compile_definitions(mixxx-lib PRIVATE QT_QML_DEBUG)
message(STATUS "Enabling QML Debugging! This poses a security risk as Mixxx will open a TCP port for debugging")
endif()

I also built locally with RelWithDebInfo, because Debug builds on Windows are so slow, that they are unusable.

@ronso0
Copy link
Member Author

ronso0 commented Jan 3, 2022

I just built main with --config RelWithDebInfo and ran the unchanged ControlObjectScriptTest.CompressingProxyManyEvents. It failed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants