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

Remove idle test and reduce media directory size #1421

Merged
merged 1 commit into from
Jan 13, 2021

Conversation

mondeja
Copy link
Collaborator

@mondeja mondeja commented Jan 12, 2021

The removed test is doing nothing. The resized file is used only in tests/test_ffmpeg_reader.py::test_ffmpeg_parse_infos to check that audio is read and the audio FPS is 44100, so 4 minutes of video are not needed. This reduces the media/ directory size from ~21mb to ~5.8mb.

  • I have formatted my code using black -t py36

@tburrows13
Copy link
Collaborator

I wouldn't say that it is doing nothing, but agreed that the value of it is very low. For reference it was added in #377.
On the other hand, removing the large file doesn't change the pip install size (media aren't included in that), and it won't reduce the git clone size either because it will still be downloaded as part of the git history.

I'm happy to remove it if you still are though.

@tburrows13 tburrows13 added the tests Related to individual tests in the test suite or running the test suite. label Jan 12, 2021
@mondeja
Copy link
Collaborator Author

mondeja commented Jan 12, 2021

The test it's not executing nothing, it's only declaring the function test_audio_reader. I've forced a exception inside the wrapped function and doesn't fails:

def test_issue_246():
    def test_audio_reader():
        raise Exception("FOO")
        video = VideoFileClip("media/video_with_failing_audio.mp4")
        subclip = video.subclip(270)
        subclip.write_audiofile(
            os.path.join(TMP_DIR, "issue_246.wav"), write_logfile=True
        )

That's the reason because I'm saying that it is doing nothing. Please, could you double check it?

@tburrows13
Copy link
Collaborator

Oh wow, I never noticed that before! You are right. I say scrap it then 👍🏼

@mondeja mondeja changed the title Remove idle test reducing media directory size Remove idle and reduce media directory size Jan 12, 2021
@mondeja mondeja changed the title Remove idle and reduce media directory size Remove idle test and reduce media directory size Jan 13, 2021
@mondeja
Copy link
Collaborator Author

mondeja commented Jan 13, 2021

Perfect, so go ahead with the merge 🎉

@mondeja mondeja merged commit 1e59939 into Zulko:master Jan 13, 2021
tburrows13 pushed a commit to tburrows13/moviepy that referenced this pull request Jan 19, 2021
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 this pull request may close these issues.

2 participants