Skip to content

Add tests for Audio module types#2927

Merged
ChrisThrasher merged 1 commit into
SFML:masterfrom
ChrisThrasher:test_audio
May 16, 2024
Merged

Add tests for Audio module types#2927
ChrisThrasher merged 1 commit into
SFML:masterfrom
ChrisThrasher:test_audio

Conversation

@ChrisThrasher

@ChrisThrasher ChrisThrasher commented Apr 4, 2024

Copy link
Copy Markdown
Member

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
Copy Markdown
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

coveralls commented Apr 11, 2024

Copy link
Copy Markdown
Collaborator

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

ChrisThrasher commented Apr 22, 2024

Copy link
Copy Markdown
Member Author

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
@codecov

codecov Bot commented May 14, 2024

Copy link
Copy Markdown

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
Copy Markdown
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
@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

No open projects
Status: Done

Development

Successfully merging this pull request may close these issues.

3 participants