Adding Acceptance Tests for compose based tests #2357
Conversation
Using an improvment of pion/datachannel, the channel opener can now set an event to be called when the DATA_CHANNEL_ACK message is recieved Resolves pion#1063 Relates to pion/datachannel#81
Codecov ReportBase: 77.55% // Head: 77.51% // Decreases project coverage by
Additional details and impacted files@@ Coverage Diff @@
## master #2357 +/- ##
==========================================
- Coverage 77.55% 77.51% -0.04%
==========================================
Files 87 87
Lines 9292 9292
==========================================
- Hits 7206 7203 -3
- Misses 1652 1654 +2
- Partials 434 435 +1
Flags with carried forward coverage won't be shown. Click here to find out more.
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. ☔ View full report at Codecov. |
|
FYI, I added E2E test before to confirm audio through WebRTC is received by browser. I feel that the test application is better to be separated from examples like the above e2e dir. |
Thanks for that test, Sean quoted it on slack and I tracked you down for this review ;-) Once this PR is ready and merged, I'll be happy to work together on "folding" your tests to the new infrastructure and get it to run both in the CI and on devs hosts.
I'd rather not create separate directories for e2e, examples, etc. and keep it flat for two reasons:
What we can do is have a naming convention for the tests directories: Have all e2e tests start with "e2e_" and examples start with "ex_", integration with "int_", system tests with "sys_" and so on. This way |
|
FYI, there is a e2e test here |
The offer side of the example now includes a -count and -wait options so the test will eventually end and run faster. It also ends sending and receiving EOF for clean "answer" exit.
|
Happy holidays! I've just pushed a couple of commits to bring into ats the pion-to-pion & e2e tests. I still need to refactor the data-channel as I learned a lot in the process. I copied the e2e test as is and in the process came to realize we need to refactor it. Today it runs on a single container and it's better to have at least two as we can separate the client from the server. From the pion-to-pion example I did a few changes:
With these changes all I needed to add is a lab.yaml file: version: '3'
services:
answer:
image: golang:1.19
volumes:
- .:/go/src/github.com/pion/webrtc
working_dir: /go/src/github.com/pion/webrtc/examples/pion-to-pion/answer
command: go run . -offer-address runner:50000
runner:
depends_on:
- answer
image: golang:1.19
volumes:
- .:/go/src/github.com/pion/webrtc
working_dir: /go/src/github.com/pion/webrtc/examples/pion-to-pion/offer
command: go run . -answer-address answer:60000 -count 3 -wait 1It's a template I still need to copy back to the data-channels. I also haven't finished work on the CI - replacing the e2e with running |
a test passes on my machine and fails as part of the workflow. this change triples the time the offer is side is waiting for a reply
as it's now part of the acceptance tests
restoring the xample to it's original code and adjusting the test spec
stv0g
changed the title
with this change the data-channels pion docker works similar to the other tests and mapping the source directly to /go/src
improve names in the ats workflow
by making sure the DOM is there before try to access it. Hoping it will make a flaky test stable.
This reverts commit 045df4c.
This reverts commit a92c400.
Using an improvment of pion/datachannel, the channel opener can now set an event to be called when the DATA_CHANNEL_ACK message is recieved Resolves pion#1063 Relates to pion/datachannel#81
added a wait loop that checks the ensures the sendChanel is open before hitting send
the code was to protected against a situation where ice gathering finished before the DOM is loaded. it never happens.
the files will be available in the action's page, and could be open using playwright trace app.
adding a script to help ensure only the answer will be printed
problem is, it's all get squeezed in the answer field..
that way I have a chance of figuring out what the error is
when buildign the data channel example
otherwise we get a permission error
|
All three tests are passing and are running in the CI. All that's left is getting the folder name right. We have |
|
What do you think about name "end-to-end-tests"? or "e2e-tests"? |
|
I like e2e as a concept but it's less fitting here. As a library, webrtc doesn't come with an "end". To test e2e we have to either include a test only frontend or one of the examples. In any case it's not truly an end-to-end test. Another reason I prefer Acceptance Test Procedure is that e2e tests are only one of the kinds of tests this infrastructure support. The infrastructure can support any black-box test we need. We can use it for interoperability tests, integration tests, component tests, functional tests, etc. |
But we do include tests with front end (browser-e2e) and tests that run an example apps (pion-to-pion). May be it is only me, but I don't understand what an acceptance test is in software development. If it is to "accept" software as working and ready for release, then all tests are acceptance tests. I am not against the name, just curios. I would only use more verbose name, like acceptance_tests, not ats. |
|
Thank you for your kind words and questions. You're right - all our tests are acceptance tests. Some of them are also unit tests (aka white box testing) and for those we follow Go's pattern. For all other acceptance tests we have this virtual lab infrastructure in a special folder. "Acceptance Tests" is the name of the workflow I added, maybe I should move unit testing there? As for the name, let me give you more context. This is the third time I'm adding this to a project. It started with Terminal7 where I used it to verify my trio of client, server and signaling server are playing well together. It has some e2e tests but most are integration tests for connection and disconnection scenarios. I then copied it to the server and wrote a test for a new CLI subcommand. I guess in this case it's an e2e test as the test starts with CLI. I believe it doesn't really matter if it's an e2e or an ABC type of test. As long as we have more tests, fewer bugs will slip through. I was once in a project where the type of tests did matter. We were developing avionics software for the F16 fighter jet and it was a very carefully engineered project. There was even a military standard for the waterfall process we were using. There I first met the ATPs - meticulously planned test flights to ensure the systems work and the bombs hit their targets. In this day and age, we don't plan like this (I guess some unlucky few still do). Any test is welcome and we should encourage contributors to add any acceptance tests for their code. ATPs are loosely defined and it's a good thing as they can cover any kind of test contributors will come up with. |
Thank you for the thorough answer. Regarding unit tests, in my opinion it is good to have separate GitHub workflow for them. Because it gives transparency on what tests fail/pass (unit or acceptance). |
This one replaces an uderscore with a dash
based on an auto-review here are my fixes
somehow, I lost the ""
In the playwright container, so the tests will run only once
daonb
changed the title
|
I left the unit test where it was and changed the directory name to |
Description
There's a black box testing pattern I've been using in my projects and I'm happy to contribute. It's based on docker compose and playwright. For now, there are three tests:
I've made as little changes as possible to the example: adding data-test-id in a couple of places and making main.go respond promptly on establishing a connection. It could be premature optimization, but waiting 5 seconds every time seems silly.