From 000a2a0d513729bd560bced898967f7cc90d5530 Mon Sep 17 00:00:00 2001 From: Xuehai Pan Date: Mon, 27 Sep 2021 05:53:36 +0800 Subject: [PATCH] Update test case for `VideoRecorder` (#2404) * Update test case for VideoRecorder * Remove closer for VideoRecorder --- .../monitoring/tests/test_video_recorder.py | 20 +++++++++++++------ gym/wrappers/monitoring/video_recorder.py | 6 ------ 2 files changed, 14 insertions(+), 12 deletions(-) diff --git a/gym/wrappers/monitoring/tests/test_video_recorder.py b/gym/wrappers/monitoring/tests/test_video_recorder.py index ce2f55fde49..cad0dd5a800 100644 --- a/gym/wrappers/monitoring/tests/test_video_recorder.py +++ b/gym/wrappers/monitoring/tests/test_video_recorder.py @@ -1,11 +1,9 @@ -import json +import gc import os -import shutil -import tempfile -import numpy as np +import time import gym -from gym.wrappers.monitoring.video_recorder import VideoRecorder, video_recorder_closer +from gym.wrappers.monitoring.video_recorder import VideoRecorder class BrokenRecordableEnv(object): @@ -27,7 +25,13 @@ def test_record_simple(): rec = VideoRecorder(env) env.reset() rec.capture_frame() + proc = rec.encoder.proc + + assert proc.poll() is None # subprocess is running + rec.close() + + assert proc.poll() is not None # subprocess is terminated assert not rec.empty assert not rec.broken assert os.path.exists(rec.path) @@ -52,7 +56,11 @@ def record(): return rec_path, proc rec_path, proc = record() - assert proc.poll() is not None + + gc.collect() # do explicit garbage collection for test + time.sleep(5) # wait for subprocess exiting + + assert proc.poll() is not None # subprocess is terminated assert os.path.exists(rec_path) f = open(rec_path) assert os.fstat(f.fileno()).st_size > 100 diff --git a/gym/wrappers/monitoring/video_recorder.py b/gym/wrappers/monitoring/video_recorder.py index e9a745c8823..1425bb0b2cd 100644 --- a/gym/wrappers/monitoring/video_recorder.py +++ b/gym/wrappers/monitoring/video_recorder.py @@ -10,16 +10,12 @@ import numpy as np from gym import error, logger -from gym.utils import closer def touch(path): open(path, "a").close() -video_recorder_closer = closer.Closer() - - class VideoRecorder(object): """VideoRecorder renders a nice movie of a rollout, frame by frame. It comes with an `enabled` option so you can still use the same code @@ -42,7 +38,6 @@ def __init__(self, env, path=None, metadata=None, enabled=True, base_path=None): self._async = env.metadata.get("semantics.async") self.enabled = enabled self._closed = False - self._recorder_id = video_recorder_closer.register(self) # Don't bother setting anything else if not enabled if not self.enabled: @@ -190,7 +185,6 @@ def close(self): self.write_metadata() # Stop tracking this for autoclose - video_recorder_closer.unregister(self._recorder_id) self._closed = True def write_metadata(self):