Closed cosmin closed 2 months ago
Let's see how long the tests take to run with this on by default.
Looks like upon test failures the build automatically calls the action-tmate
action which then waits for a while for someone to connect via SSH to debug. Unless someone is in the middle of debugging something that seems suboptimal, should we turn this off for now, by deleting the debug
environment and only re-adding it as needed?
Integration tests should pass after #1379
should we turn this off for now, by deleting the
debug
environment and only re-adding it as needed?
Yes! I should have deleted it long ago. I left it on by mistake.
should we turn this off for now, by deleting the
debug
environment and only re-adding it as needed?Yes! I should have deleted it long ago. I left it on by mistake.
Done. I'll rerun the tests here.
So here are some factors that I believe we have to sift through in evaluating the timing:
Happy to debate any of these if you disagree.
Two build workflow runs for comparison:
Another way to look at this, which may be simpler, is to find the logs of the packager_test_py run, which contain a standalone wall-clock time:
1/21 Test #1: packager_test_py .................***Failed 5.13 sec
1/21 Test #1: packager_test_py .................***Failed 9.99 sec
1/21 Test #1: packager_test_py .................***Failed 16.53 sec
1/21 Test #1: packager_test_py .................***Failed 5.79 sec
The error from windows logs:
Process not started
D:/a/shaka-packager/shaka-packager/build/packager/packager_test.py
[unknown error]
I suspect we need to have the test command involve "python3" or something, at least on Windows. We are probably relying on the shebang line for Linux & macOS.
Does packager_test_py
quit on the first failure? If so, the timings above are garbage until you merge your fix from #1379.
I think the problem is that the integration tests rely on reaching into the source tree to get the test media, and make assumptions about the hardcoded path to the source tree.
1: Test command: D:\a\shaka-packager\shaka-packager\build\packager\packager_test.py
I think the test command needs to include "python3" and avoid using the shebang.
The test is defined using
add_test (NAME packager_test_py
COMMAND ${PYTHON_EXECUTABLE} packager_test.py
WORKING_DIRECTORY ${CMAKE_CURRENT_BINARY_DIR}
)
so in theory it should be using python3, but it doesn't look like anything sets the Python executable. I think we need to use FindPython
and then only add the test if an interpreter is found.
Down to only one test failing on Windows shared builds, with the app failing to recognized the --strip_parameter_set_nalus
flag, although I think this failure exposes a larger problem. Due to default symbol visibility rules on Windows any flags that are declared within the packager library rather than the packager app are not going to be visible to the app, and therefore ignored by Abseil. All of these flags will likely need SHAKA_EXPORT
Still haven't found a good solution to the abseil flags from the packager shared library not being recognized by the packager app binary. So I went back to fixing what the tests originally tried to do, and skip them if it was built as a shared library. We can circle back to fixing these flags but meanwhile we should enable the integration tests as part of the build.
They can still be skipped by passing
-DSKIP_INTEGRATION_TESTS=ON
for the build configuration.