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

Add tests for Audio module types #2927

Merged
merged 1 commit into from May 16, 2024
Merged

Conversation

ChrisThrasher
Copy link
Member

@ChrisThrasher ChrisThrasher commented Apr 4, 2024

Description

This will remain a draft PR until #2749 is merged as to avoid introducing more merge conflicts into that PR.

I first wrote these tests a long time ago but just realized that we can use the trick as we do for the tests that require a display. Only the macOS jobs include an audio device so that's the only OS on which we can run these tests. Not great but better than nothing. These tests should still pass on everyone's own development machine since we all use computers with sound cards.

Coverage of these Audio module types is not perfect but we can improve that later once this first tranche of tests is merged.

@ChrisThrasher
Copy link
Member Author

https://github.com/SFML/SFML/actions/runs/8573099270/job/23497199870#step:19:141

For some reason after adding new sf::Music tests that load sounds from disk, the tests now hang indefinitely on our Arm Mac runners. On my own Arm Mac these tests work perfectly fine completing in <2 seconds. I'd guess there is some difference in hardware between the macOS 12 x86 runners and the macOS 14 Arm runners that is causing this. Not sure how to go about debugging this since I can't reproduce it locally.

@coveralls
Copy link
Collaborator

coveralls commented Apr 11, 2024

Pull Request Test Coverage Report for Build 9116395159

Details

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage increased (+3.4%) to 55.157%

Totals Coverage Status
Change from base Build 9089388041: 3.4%
Covered Lines: 11424
Relevant Lines: 19495

💛 - Coveralls

@ChrisThrasher ChrisThrasher force-pushed the test_audio branch 4 times, most recently from 29e5a25 to 86c3efd Compare April 17, 2024 01:35
@ChrisThrasher ChrisThrasher force-pushed the test_audio branch 2 times, most recently from 0db24e6 to 3c3f4d3 Compare April 22, 2024 17:35
@ChrisThrasher ChrisThrasher marked this pull request as ready for review April 22, 2024 17:35
@ChrisThrasher
Copy link
Member Author

ChrisThrasher commented Apr 22, 2024

I'm marking this as ready for review because these tests catch a bug in the current implementation of #2749 and detect some minor behavior changes. I think it's worth merging these tests first so that the Miniaudio PR can benefit from some better automated testing.

@ChrisThrasher ChrisThrasher force-pushed the test_audio branch 3 times, most recently from 483c4b3 to 1b78632 Compare April 22, 2024 21:10
Copy link

codecov bot commented May 14, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 45.01%. Comparing base (6eaf300) to head (360d780).
Report is 66 commits behind head on master.

❗ Current head 360d780 differs from pull request most recent head 27c48ce. Consider uploading reports for the commit 27c48ce to get more accurate results

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #2927      +/-   ##
==========================================
+ Coverage   43.65%   45.01%   +1.36%     
==========================================
  Files         253      253              
  Lines       20940    20947       +7     
  Branches     5139     5160      +21     
==========================================
+ Hits         9142     9430     +288     
+ Misses      10785    10469     -316     
- Partials     1013     1048      +35     

see 30 files with indirect coverage changes


Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update a61eb6a...27c48ce. Read the comment docs.

@ChrisThrasher
Copy link
Member Author

With the changes from #2966 I was able to enable the audio tests on Windows as well! This PR is ready to merge once the BuildBot is updated to set -DSFML_RUN_AUDIO_DEVICE_TESTS=OFF for the jobs that are currently failing. Those machines much lack the hardware required to run these tests just like GitHub's Linux runners lack such hardware.

@ChrisThrasher ChrisThrasher force-pushed the test_audio branch 2 times, most recently from 84e9db1 to 963f14c Compare May 16, 2024 15:46
@ChrisThrasher ChrisThrasher merged commit 9722fb3 into SFML:master May 16, 2024
113 checks passed
@ChrisThrasher ChrisThrasher deleted the test_audio branch May 16, 2024 18:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

None yet

3 participants