cta-wave / dpctf-tests

Repo for DPCTF Tests. We prefer to keep the Tests separated from Test Runner
Other
2 stars 5 forks source link

action stalls on representation change in switching set tests #117

Closed jpiesing closed 1 year ago

jpiesing commented 1 year ago

While analysing failures, it was noticed that the action stalls on "representation_change" and is not updated for the test of the test run. This results in the OF no longer analysing the current Time for the rest of the test run. See https://github.com/cta-wave/device-observation-framework/issues/57#issuecomment-1705593677 .

image

We should confirm if this is a bug in the test.

louaybassbouss commented 1 year ago

@jpiesing I discussed this with @FritzHeiden, we think the OF should continue monitoring the currentTime because a: representation_change means, the video is still playing, but there was a representation change. We can discuss this in the call shortly.

yanj-github commented 1 year ago

I think it should be an easy change. OF can change to carry on with monitoring the currentTime when "a: representation_change". So it will be when "s: playing" and "a: play" or "a: representation_change". Is there any other case that we should consider as well please?

FritzHeiden commented 1 year ago

The two mentioned are the only ones to consider

yanj-github commented 1 year ago

Thanks @FritzHeiden and @louaybassbouss @jpiesing are you happy with the discussion above observing current time match when "a: representation_change" as well? We will make the change on observation. I am happy to managing issue here but feel free to add to OF repo if you would prefer.

jpiesing commented 1 year ago

I'm OK with this change but please remember that it may be hiding other problems which we won't know until we have an implementation with this fix and have run that and looked at the result. We may find things which suggest this isn't the best answer - I hope not but we may.

yanj-github commented 1 year ago

Changes made and now avalible on OF main branch.

yanj-github commented 1 year ago

@jpiesing this has been resolved on OF I think this issue can be closed.

jpiesing commented 1 year ago

The switching set tests are failing badly (which this was partly concealing) but that's a new issue.