Create test for multiple matches bug #9
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Sorry I've not been able to get to this. What we've been doing in (in related project vcdimager) when we have large test files is to put them somewhere else and then have the tests run optionally depending on whether these are around. See http://cvs.savannah.gnu.org/viewvc/vcdimager/vcdimager/vcd_examples/README?view=markup I'll set this up as a separate project, maybe a git subproject and have a way to allow developers to test with this. So in sum, although it's going to take a while to get to it, rest assured I will get to it and appreciate your creating the test. and run the test option |
|
Any updates on this? Anything I can do to help? |
|
Sorry for the delay. The only thing that I'd request is to remove cdda2.bin and make sure a test for it runs only if it happens to be where it currently is located. Then if you put someplace I can get to, I'll arrange to add it to the existing set of optional test cases for those libcdio developers and others who have disk space and want to try the full set of tests. (Putting a large in the repo forcing everyone to download it, is probably not user friendly) As soon as this is done, I promise I'll merge. |
|
I updated the commit to remove the file and only run the test when data file is present. The data file can be found here: |
|
Thanks. I tried building from you the multiple matches branch of your repo and get this error when running There shouldn't be a rule I believe for cdda2.bin since that is optional, right? The link you sent seems to be a cue sheet, not a huge file. And I am guessing you meant to have two files, both the cue and the data right? Or are you reusing a cue sheet? |
The test data is pretty large, but since the bug only happens in the places where paranoia tries to merge blocks we need to have at least two blocks and these are around 2.5MB.
|
Yeah, sorry you're right, I forgot to update the Makefile.am file, updated the commit. The link is a 5.3MB binary file for me, not sure what is going on there. I left the cue sheet for the data file in the repository, since it's only 176 bytes. |
|
Have the binary file now. Didn't realize I had to click on the link in a browser and then download from there. (Used wget). Checking now... |
|
Sigh. Now the new test is failing. The error messages say something about missing drive capabilities but that shouldn't happen on a cue/bin disc image. |
|
The test is supposed to fail, this is based on the master branch. If you merge this to one of the fix branches (and recompile of course), the tests will pass. I did it on the master branch to make sure that the test actually fails with the old code. As far as I can tell, the "option not supported by drive" output doesn't really matter, it also happens with the other tests in |
|
Ok. I'll try with merging the other MR a little bit later and test and then merge this. Thanks. |
|
Thanks for all of the good work, and your patience. I'm sorry we couldn't and didn't get to this sooner. |
ghost
deleted the
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The test data is pretty large, but since the bug only happens in the places
where paranoia tries to merge blocks we need to have at least two blocks and
these are around 2.5MB.