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

Assertions in tests are very fragile #1114

Closed
mgaitan opened this issue Mar 28, 2020 · 2 comments · Fixed by #1124
Closed

Assertions in tests are very fragile #1114

mgaitan opened this issue Mar 28, 2020 · 2 comments · Fixed by #1124
Assignees
Labels
tests Related to individual tests in the test suite or running the test suite.

Comments

@mgaitan
Copy link
Collaborator

mgaitan commented Mar 28, 2020

Doing tests for #1076 I've realized most of the tests that currently exists are very naive and just verify general conditions of the result, like "result is a file" or, worse, that the executed code doesn't fail (doesn't raise an exception) without any assertion on the result.

We should improve this by checking actual values of the result. Most of the time, this is possible comparing frames as numpy arrays. For example, to check a "time mirror" effect, the first frame of the input clip should be the last one in the output one.

@mgaitan mgaitan changed the title Assertions in test are very fragile Assertions in tests are very fragile Mar 28, 2020
@tburrows13
Copy link
Collaborator

I've been thinking exactly the same thing. Something to get round to at some point (I'm happy to do it).

@tburrows13 tburrows13 self-assigned this Mar 31, 2020
@tburrows13 tburrows13 added the tests Related to individual tests in the test suite or running the test suite. label Mar 31, 2020
@tburrows13
Copy link
Collaborator

I'm working on a framework for this now, I'll open a WIP PR soon.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
tests Related to individual tests in the test suite or running the test suite.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants