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

more e2e tests #112

Merged
merged 3 commits into from
Sep 5, 2023
Merged

more e2e tests #112

merged 3 commits into from
Sep 5, 2023

Conversation

ortex
Copy link
Member

@ortex ortex commented Aug 9, 2023

add e2e TestEnd2End_Blocking for ClockInternal clock source; split default e2e test for with/without fec

#102 #104

@ortex ortex added the ready for review Pull request can be reviewed label Aug 9, 2023
@coveralls
Copy link
Collaborator

Coverage Status

coverage: 95.037%. remained the same when pulling 3d7e60f on more_e2e_tests into 895c183 on main.

Copy link
Member

@gavv gavv left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! One small comment, not even related to PR itself.

roc/e2e_test.go Outdated
samplesCnt := 100
testSamples := generateTestSamples(samplesCnt)

var interval = time.Second / time.Duration(44100/samplesCnt)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: Let's use sample rate from config, instead of 44100?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done!

@gavv gavv added needs revision Pull request should be revised by its author and removed ready for review Pull request can be reviewed labels Aug 30, 2023
@ortex ortex force-pushed the more_e2e_tests branch 5 times, most recently from bdec9e1 to 98d9865 Compare September 5, 2023 18:31
@ortex
Copy link
Member Author

ortex commented Sep 5, 2023

@gavv Could you have a look?

fixed the discussion and set go-version to 1.20 instead of the latest because our ci pipeline fails with new go 1.21, see https://github.com/roc-streaming/roc-go/actions/runs/6067814352 I will create a separate issue for that

Btw, need to use '1.20' as go version for action instead of 1.20, because 1.20 is treated as 1.2 actions/setup-go#326 (comment)

@ortex ortex added ready for review Pull request can be reviewed and removed needs revision Pull request should be revised by its author labels Sep 5, 2023
@gavv gavv merged commit 6212890 into main Sep 5, 2023
8 checks passed
@gavv gavv deleted the more_e2e_tests branch September 5, 2023 21:35
@gavv
Copy link
Member

gavv commented Sep 5, 2023

Thanks! Damn, it was float!

@gavv gavv removed the ready for review Pull request can be reviewed label Sep 20, 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.

3 participants