Update test case for VideoRecorder (#2404)

* Update test case for VideoRecorder

* Remove closer for VideoRecorder

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

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):