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

test/call.c: extend test_call_hold_resume #2800

Merged
merged 1 commit into from
Nov 7, 2023

Conversation

maximilianfridrich
Copy link
Contributor

@maximilianfridrich maximilianfridrich commented Nov 6, 2023

Updates the test_call_hold_resume test by initiating a call and offering the streams as sendonly. The test verifies that this does not trigger an UA_EVENT_CALL_HOLD on the receiving side of this offer. It then does a few hold/resumes from both sides and verifies the events UA_EVENT_CALL_HOLD and UA_EVENT_CALL_RESUME are only triggered when they should.

@maximilianfridrich maximilianfridrich changed the title test/call.c: add test_call_mdir test/call.c: extend test_call_hold_resume Nov 6, 2023
@cspiel1
Copy link
Collaborator

cspiel1 commented Nov 6, 2023

Looks good to me.

@cspiel1
Copy link
Collaborator

cspiel1 commented Nov 6, 2023

In a future PR we should make the counter n_... to be integer instead of uint.
-1 means do not compare the value.

@alfredh alfredh merged commit 305d0bb into baresip:main Nov 7, 2023
16 checks passed
@alfredh
Copy link
Collaborator

alfredh commented Nov 7, 2023

This test became unstable after the merge:

https://github.com/baresip/baresip/actions/runs/6783024753/job/18436437302

can you please investigate ?

@maximilianfridrich
Copy link
Contributor Author

maximilianfridrich commented Nov 7, 2023

@alfredh I'm not sure if I'm interpreting the linked output correctly, but it looks like it found a data race when running test_call_100rel_audio. I don't know how I could've caused a data race with this change, and I didn't touch test_call_100rel_audio.

@cspiel1
Copy link
Collaborator

cspiel1 commented Nov 8, 2023

I will have a look.

@cspiel1
Copy link
Collaborator

cspiel1 commented Nov 13, 2023

This test became unstable after the merge:

https://github.com/baresip/baresip/actions/runs/6783024753/job/18436437302

can you please investigate ?

This #2809 should directly solve the sanitizer warning.

maximilianfridrich added a commit to maximilianfridrich/baresip that referenced this pull request Dec 18, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants