-
Notifications
You must be signed in to change notification settings - Fork 270
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
WIP: A few more Frame tests #446
base: develop
Are you sure you want to change the base?
Conversation
- Some tests are commented out; behavior is strange / unexpected / inconsistent at times; this probably needs to be fixed, thus do not capture it in test for now.
- Strange / unexpected / probably wrong behavior; this tests needs to be expanded once there's a clear API
- This test needs to be expanded to test how (if) this affects underlying audio data.
Regarding the failing CI: This is due to me using |
What is the issue with the existing tests? There is no clear explanation given. |
The existing tests cover almost none of |
Our tests desperately need more coverage, so I'm very much in favor of that under any circumstances. As far as the Ubuntu 16.04 failures, unfortunately that release is still supported for another year+ (not 2 months, as I'd previously said -- I was off by a year), so our tests need to be compatible. If there's a way to use #ifndef REQUIRE
#define REQUIRE
#endif (Edit: Corrected to simplify even further.) Unfortunately we won't get an updated coverage report until the CI run passes all jobs. |
I just pushed a commit to do exactly that, we'll see if Travis likes it... |
That worked, just waiting on the rest of the builds to finish so we can get updated coverage stats. |
Codecov Report
@@ Coverage Diff @@
## develop #446 +/- ##
===========================================
- Coverage 49.45% 42.50% -6.96%
===========================================
Files 129 129
Lines 10261 13299 +3038
===========================================
+ Hits 5075 5653 +578
- Misses 5186 7646 +2460
Continue to review full report at Codecov.
|
e428d2d
to
92e52a7
Compare
Fixed the 44.2 to 44.1 :) |
Looking at the full coverage map for But there is still plenty of other stuff we could be testing, like (just walking down the source from top to bottom...):
(...I'm just throwing that list out there as a sort of general status-check. It's certainly not anyone's to-do list. Or, maybe it's everyone's to-do list. Meaning, the list is there — and please feel free to amend or correct it — for anyone who'd like to contribute tests for any of those branches. I'm hoping it'll help me guilt myself into writing at least a few of them, too.) |
Merge conflicts have been detected on this PR, please resolve. |
I've expanded the test suite for the
Frame
class a bit, and also corrected some (seemingly?) outdated comments inFrame.h
. The tests still do not cover most of what aFrame
can do. A lot of the behavior which I'd like to test more is actually quite ... surprising, though. Issue about that: #447Depending on the conclusion of the issue I'll further extend these tests to capture either the existing behavior as good as possible or the IMO more consistent behavior. Therefore I mark this as "Work In Process" for now :)