fishjam-dev / fishjam

General purpose media server. Supports WebRTC, HLS, RTSP, SIP
https://fishjam-dev.github.io/fishjam-docs/
Apache License 2.0
197 stars 12 forks source link

[RTC-379] Fix "list all rooms" test. Make RoomControllerTest sync. #114

Closed Rados13 closed 10 months ago

Rados13 commented 10 months ago

Acknowledging the stipulations set forth:

This PR fixes two tests:

In addition, in component_controller_test.exs, the naming for connection was unified, so in all test cases, we use the name conn instead of mixing conn and room_conn

mickel8 commented 10 months ago

I am not sure but we could make room_conn and conn distinction on purpose. @Karolk99 do you remember why?

Karolk99 commented 10 months ago

I am not sure but we could make room_conn and conn distinction on purpose. @Karolk99 do you remember why?

I'm not sure. I think I made those tests analogical to the setup implementation. Sometimes there is a problem with reusing the connection (Plug.Conn.AlreadySentError). @Rados13 why didn't you change room_conn in setup?

Rados13 commented 10 months ago

I am not sure but we could make room_conn and conn distinction on purpose. @Karolk99 do you remember why?

I'm not sure. I think I made those tests analogical to the setup implementation. Sometimes there is a problem with reusing the connection (Plug.Conn.AlreadySentError). @Rados13 why didn't you change room_conn in setup?

I didn't change it because of the error you mentioned. If I understand this Phoenix mechanism correctly, we should use the output connection from the previous request as input in the next request. But because on_exit is called after tests, we can't use it that way. But I am not sure about it.

codecov[bot] commented 10 months ago

Codecov Report

Merging #114 (653ea54) into main (7e03dcb) will not change coverage. Report is 3 commits behind head on main. The diff coverage is n/a.

@@           Coverage Diff           @@
##             main     #114   +/-   ##
=======================================
  Coverage   86.33%   86.33%           
=======================================
  Files          50       50           
  Lines         922      922           
=======================================
  Hits          796      796           
  Misses        126      126           

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 7e03dcb...653ea54. Read the comment docs.