-
Notifications
You must be signed in to change notification settings - Fork 517
[FIX] cargo tests failing on windows #1704
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
Conversation
a89c6ab to
4c3f4fb
Compare
4c3f4fb to
c7eea7e
Compare
|
CCExtractor CI platform finished running the test files on linux. Below is a summary of the test results, when compared to test for commit 2260165...:
All tests passing on the master branch were passed completely. NOTE: The following tests have been failing on the master branch as well as the PR:
Congratulations: Merging this PR would fix the following tests:
Check the result page for more info. |
|
CCExtractor CI platform finished running the test files on windows. Below is a summary of the test results, when compared to test for commit 715597e...:
All tests passing on the master branch were passed completely. NOTE: The following tests have been failing on the master branch as well as the PR:
Congratulations: Merging this PR would fix the following tests:
Check the result page for more info. |
prateekmedia
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM! Thanks.
In raising this pull request, I confirm the following (please check boxes):
My familiarity with the project is as follows (check one):
Cargo tests have been failing on Windows on the master branch for a while now. The primary reason seems to be differences in linking behaviour between the GNU and MSVC ABIs, see here for more info.
The main issue was that when running unit tests, the linker that MSVC uses couldn't resolve several external C symbols, primarily these. This makes sense as when running unit tests, the C side of the codebase never comes into play. To address this, I added a
cfg_ifblock that provides mock implementations during test runs so that the tests can run without requiring linkage to C symbols.I also noticed that there was an extern declaration in
lib_ccxrforMPEG_CLOCK_FREQUENCY, after consulting with @steel-bucket I realized that this is was an oversight as thelib_ccxrcrate is meant to be a clean idiomatic rust layer. To fix this, I added a new field to GLOBAL_TIMING_INFO and ensured the MPEG clock frequency is passed explicitly through FFI.With these changes, all unit tests now run successfully on Windows. This should help with fixing any windows specific bugs in upcoming PRs.