Fix oppia#17730: Screen-Record LHCI Puppeteer Scripts (oppia#18714)

* first commit * add core functionality * fix linter errors * install ffmpeg before LHCI test * re-test LHCI functionality * fix upload location of ffmpeg video * fix resulting video path 2 * fix resulting video path 3 * fix style errors * fix ffmpeg subprocess management 1 * fix ffmpeg subprocess output 2 * Merge remote-tracking branch 'upstream/develop/ into lhci-puppeteer-record. * use puppeter-screen-recorder for lhci * fix linter issues 1 * remove unused code * add puppeteer recorder install * add backend test coverage and install via package.json * fix backend error for 2-argument print * fix backend lhci test 2 * fix style 1 * fix lhci backend test 3 * fix lhci backend tesets 4 * fix type error from previous commit * fix lhci backend tests 4 * fix lhci backend tests 5 * add full lhci backend test coverage * add extra tests for full lhci coverage * fix path checking * delete fake output file between tests * create separate success and failure recording tests * use mock isfile function in puppeteer recording tests * add chrome env variable for lhci accessibility check * add test case for main function * fix mypy failure * fix main --record test 1 * address PR comments * remove common set chrome bin env variable * fix style 1 * correct build expected args * resolve comments 2 * fix style issue --------- Co-authored-by: Cary Xiao <[email protected]>
Ash-2k3 · Aug 23, 2023 · 95e20a2 · 95e20a2
1 parent 5bf25ee
commit 95e20a2
Show file tree

Hide file tree

Showing 5 changed files with 191 additions and 5 deletions.
diff --git a/.github/workflows/e2e_lighthouse_performance_acceptance_tests.yml b/.github/workflows/e2e_lighthouse_performance_acceptance_tests.yml
@@ -276,7 +276,13 @@ jobs:
         run: python -m scripts.install_chrome_for_ci
       - name: Run Lighthouse performance checks shard
         if: startsWith(github.head_ref, 'update-changelog-for-release') == false
-        run: python -m scripts.run_lighthouse_tests --mode performance --shard ${{ matrix.shard }} --skip_build
+        run: python -m scripts.run_lighthouse_tests --mode performance --shard ${{ matrix.shard }} --skip_build --record_screen
+      - name: Uploading puppeteer video as Artifacts
+        if: ${{ failure() }}
+        uses: actions/upload-artifact@v3
+        with: 
+          name: lhci-puppeteer-video
+          path: /home/runner/work/oppia/lhci-puppeteer-video/video.mp4
       - name: Report failure if failed on oppia/oppia develop branch
         if: ${{ failure() && github.event_name == 'push' && github.repository == 'oppia/oppia' && github.ref == 'refs/heads/develop'}}
         uses: ./.github/actions/send-webhook-notification

diff --git a/core/tests/puppeteer/lighthouse_setup.js b/core/tests/puppeteer/lighthouse_setup.js
@@ -19,6 +19,8 @@
 var FirebaseAdmin = require('firebase-admin');
 const process = require('process');
 const puppeteer = require('puppeteer');
+const { PuppeteerScreenRecorder } = require('puppeteer-screen-recorder');
+
 
 const ADMIN_URL = 'http://127.0.0.1:8181/admin';
 const CREATOR_DASHBOARD_URL = 'http://127.0.0.1:8181/creator-dashboard';
@@ -389,6 +391,33 @@ const main = async function() {
     width: 1920,
     height: 1080
   });
+
+  var recorder = null;
+  let record = process.argv[2] && process.argv[2] === '-record';
+  let videoPath = process.argv[3];
+  if (record && videoPath) { // Start recording via puppeteer-screen-recorder.
+    const Config = {
+      followNewTab: true,
+      fps: 25,
+      ffmpeg_Path: null,
+      videoFrame: {
+        width: 1920,
+        height: 1080,
+      },
+      videoCrf: 18,
+      videoCodec: 'libx264',
+      videoPreset: 'ultrafast',
+      videoBitrate: 1000,
+      autopad: {
+        color: 'black' | '#35A5FF',
+      },
+      aspectRatio: '16:9',
+    };
+    recorder = new PuppeteerScreenRecorder(page, Config);
+    // Create directory for video in opensource.
+    await recorder.start(videoPath);
+  }
+
   await login(browser, page);
   await getExplorationEditorUrl(browser, page);
 
@@ -408,6 +437,9 @@ const main = async function() {
       skillEditorUrl,
     ].join('\n')
   );
+  if (record) {
+    await recorder.stop();
+  }
   await page.close();
   process.exit(0);
 };

diff --git a/package.json b/package.json
@@ -102,6 +102,7 @@
     "postcss-html": "^1.3.0",
     "protoc-gen-ts": "^0.3.4",
     "puppeteer": "^13.0.0",
+    "puppeteer-screen-recorder": "2.1.2",
     "reflect-metadata": "^0.1.12",
     "rtlcss": "^3.5.0",
     "sourcemapped-stacktrace": "^1.1.11",

diff --git a/scripts/run_lighthouse_tests.py b/scripts/run_lighthouse_tests.py
@@ -69,16 +69,37 @@
 _PARSER.add_argument(
     '--shard', help='Sets the shard for the lighthouse tests',
     required=True, choices=['1', '2'])
+
 _PARSER.add_argument(
     '--skip_build', help='Sets whether to skip webpack build',
     action='store_true')
 
+_PARSER.add_argument(
+    '--record_screen', help='Sets whether LHCI Puppeteer script is recorded',
+    action='store_true')
+
 
-def run_lighthouse_puppeteer_script() -> None:
-    """Runs puppeteer script to collect dynamic urls."""
+def run_lighthouse_puppeteer_script(record: bool = False) -> None:
+    """Runs puppeteer script to collect dynamic urls.
+
+    Args:
+        record: bool. Set to True to record the LHCI puppeteer script
+            via puppeteer-screen-recorder and False to not. Note that
+            puppeteer-screen-recorder must be separately installed to record.
+    """
     puppeteer_path = (
         os.path.join('core', 'tests', 'puppeteer', 'lighthouse_setup.js'))
     bash_command = [common.NODE_BIN_PATH, puppeteer_path]
+    if record:
+        # Add arguments to lighthouse_setup that enable video recording.
+        bash_command.append('-record')
+        dir_path = os.path.join(os.getcwd(), '..', 'lhci-puppeteer-video')
+        if not os.path.exists(dir_path):
+            os.mkdir(dir_path)
+        video_path = os.path.join(dir_path, 'video.mp4')
+        bash_command.append(video_path)
+        print('Starting LHCI Puppeteer script with recording.')
+        print('Video Path:' + video_path)
 
     process = subprocess.Popen(
         bash_command, stdout=subprocess.PIPE, stderr=subprocess.PIPE)
@@ -90,6 +111,8 @@ def run_lighthouse_puppeteer_script() -> None:
             # print it.
             export_url(line.decode('utf-8'))
         print('Puppeteer script completed successfully.')
+        if record:
+            print('Resulting puppeteer video saved at %s' % video_path)
     else:
         print('Return code: %s' % process.returncode)
         print('OUTPUT:')
@@ -101,6 +124,8 @@ def run_lighthouse_puppeteer_script() -> None:
         # print it.
         print(stderr.decode('utf-8'))
         print('Puppeteer script failed. More details can be found above.')
+        if record:
+            print('Resulting puppeteer video saved at %s' % video_path)
         sys.exit(1)
 
 
@@ -226,7 +251,7 @@ def main(args: Optional[List[str]] = None) -> None:
             skip_sdk_update_check=True,
             env=env))
 
-        run_lighthouse_puppeteer_script()
+        run_lighthouse_puppeteer_script(parsed_args.record_screen)
         run_lighthouse_checks(lighthouse_mode, parsed_args.shard)
 
 

diff --git a/scripts/run_lighthouse_tests_test.py b/scripts/run_lighthouse_tests_test.py
@@ -64,7 +64,6 @@ class RunLighthouseTestsTests(test_utils.GenericTestBase):
 
     def setUp(self) -> None:
         super().setUp()
-
         self.print_arr: list[str] = []
         def mock_print(msg: str) -> None:
             self.print_arr.append(msg)
@@ -82,6 +81,11 @@ def mock_print(msg: str) -> None:
                 LIGHTHOUSE_CONFIG_FILENAMES[LIGHTHOUSE_MODE_PERFORMANCE]['1']),
             '--max-old-space-size=4096'
         ]
+        # Arguments to record in lighthouse_setup.js.
+        self.extra_args = [
+            '-record',
+            os.path.join(os.getcwd(), '..', 'lhci-puppeteer-video', 'video.mp4')
+        ]
 
         def mock_context_manager() -> MockCompilerContextManager:
             return MockCompilerContextManager()
@@ -122,6 +126,7 @@ def communicate(self) -> tuple[bytes, bytes]:   # pylint: disable=missing-docstr
 
         def mock_popen(*unused_args: str, **unused_kwargs: str) -> MockTask:  # pylint: disable=unused-argument
             return MockTask()
+
         swap_popen = self.swap_with_checks(
             subprocess, 'Popen', mock_popen,
             expected_args=((self.puppeteer_bash_command,),))
@@ -158,6 +163,67 @@ def mock_popen(*unused_args: str, **unused_kwargs: str) -> MockTask:  # pylint:
             'Puppeteer script failed. More details can be found above.',
             self.print_arr)
 
+    def test_puppeteer_script_succeeds_when_recording_succeeds(self) -> None:
+        class MockTask:
+            returncode = 0
+            def communicate(self) -> tuple[bytes, bytes]:   # pylint: disable=missing-docstring
+                return (
+                    b'https://oppia.org/create/4\n' +
+                    b'https://oppia.org/topic_editor/4\n' +
+                    b'https://oppia.org/story_editor/4\n' +
+                    b'https://oppia.org/skill_editor/4\n',
+                    b'Task output.')
+
+        def mock_popen(*unused_args: str, **unused_kwargs: str) -> MockTask:  # pylint: disable=unused-argument
+            return MockTask()
+
+        swap_isfile = self.swap(os.path, 'isfile', lambda _: True)
+        swap_popen = self.swap_with_checks(
+            subprocess, 'Popen', mock_popen,
+            expected_args=((self.puppeteer_bash_command + self.extra_args,),))
+
+        with self.print_swap, swap_popen, swap_isfile:
+            run_lighthouse_tests.run_lighthouse_puppeteer_script(record=True)
+
+        self.assertIn(
+            'Puppeteer script completed successfully.', self.print_arr)
+        self.assertIn(
+            'Starting LHCI Puppeteer script with recording.', self.print_arr)
+        self.assertIn(
+            'Resulting puppeteer video saved at %s' % self.extra_args[1],
+            self.print_arr)
+
+    def test_puppeteer_script_fails_when_recording_succeeds(self) -> None:
+        class MockTask:
+            returncode = 1
+            def communicate(self) -> tuple[bytes, bytes]:   # pylint: disable=missing-docstring
+                return (
+                    b'https://oppia.org/create/4\n' +
+                    b'https://oppia.org/topic_editor/4\n' +
+                    b'https://oppia.org/story_editor/4\n' +
+                    b'https://oppia.org/skill_editor/4\n',
+                    b'ABC error.')
+
+        def mock_popen(*unused_args: str, **unused_kwargs: str) -> MockTask:  # pylint: disable=unused-argument
+            return MockTask()
+
+        swap_isfile = self.swap(os.path, 'isfile', lambda _: True)
+        swap_popen = self.swap_with_checks(
+            subprocess, 'Popen', mock_popen,
+            expected_args=((self.puppeteer_bash_command + self.extra_args,),))
+
+        with self.print_swap, self.swap_sys_exit, swap_popen, swap_isfile:
+            run_lighthouse_tests.run_lighthouse_puppeteer_script(record=True)
+
+        self.assertIn('Return code: 1', self.print_arr)
+        self.assertIn('ABC error.', self.print_arr)
+        self.assertIn(
+            'Puppeteer script failed. More details can be found above.',
+            self.print_arr)
+        self.assertIn(
+            'Resulting puppeteer video saved at %s' % self.extra_args[1],
+            self.print_arr)
+
     def test_run_webpack_compilation_successfully(self) -> None:
         swap_isdir = self.swap_with_checks(
             os.path, 'isdir', lambda _: True, expected_kwargs=[])
@@ -267,6 +333,7 @@ def communicate(self) -> tuple[bytes, bytes]:   # pylint: disable=missing-docstr
                     b'No error.')
         def mock_popen(*unused_args: str, **unused_kwargs: str) -> MockTask:  # pylint: disable=unused-argument
             return MockTask()
+
         swap_popen = self.swap(
             subprocess, 'Popen', mock_popen)
         swap_run_lighthouse_tests = self.swap_with_checks(
@@ -359,3 +426,58 @@ def mock_popen(*unused_args: str, **unused_kwargs: str) -> MockTask:  # pylint:
             self.print_arr)
         self.assertIn(
             'Puppeteer script completed successfully.', self.print_arr)
+
+    def test_main_function_calls_puppeteer_record(self) -> None:
+        class MockTask:
+            returncode = 0
+            def communicate(self) -> tuple[bytes, bytes]:   # pylint: disable=missing-docstring
+                return (
+                    b'Task output',
+                    b'No error.')
+        env = os.environ.copy()
+        env['PIP_NO_DEPS'] = 'True'
+        # Set up pseudo-chrome path env variable.
+        for path in common.CHROME_PATHS:
+            if os.path.isfile(path):
+                env['CHROME_BIN'] = path
+                break
+        swap_dev_appserver = self.swap_with_checks(
+            servers, 'managed_dev_appserver',
+            lambda *unused_args, **unused_kwargs: MockCompilerContextManager(),
+            expected_kwargs=[{
+                'port': GOOGLE_APP_ENGINE_PORT,
+                'log_level': 'critical',
+                'skip_sdk_update_check': True,
+                'env': env
+            }])
+        swap_run_puppeteer_script = self.swap_with_checks(
+            run_lighthouse_tests, 'run_lighthouse_puppeteer_script',
+            lambda _: None,
+            expected_args=((True,),))
+        swap_run_lighthouse_tests = self.swap_with_checks(
+            run_lighthouse_tests, 'run_lighthouse_checks',
+            lambda *unused_args: None, expected_args=(('performance', '1'),))
+        def mock_popen(*unused_args: str, **unused_kwargs: str) -> MockTask:  # pylint: disable=unused-argument
+            return MockTask()
+        swap_popen = self.swap(
+            subprocess, 'Popen', mock_popen)
+        swap_isdir = self.swap(
+            os.path, 'isdir', lambda _: True)
+        swap_build = self.swap_with_checks(
+            build, 'main', lambda args: None,
+            expected_kwargs=[{'args': []}])
+        swap_emulator_mode = self.swap(constants, 'EMULATOR_MODE', False)
+        swap_popen = self.swap(
+            subprocess, 'Popen', mock_popen)
+        swap_isdir = self.swap(
+            os.path, 'isdir', lambda _: True)
+
+        with swap_popen, self.swap_webpack_compiler, swap_isdir, swap_build:
+            with self.swap_elasticsearch_dev_server, swap_dev_appserver:
+                with self.swap_ng_build, swap_emulator_mode, self.print_swap:
+                    with self.swap_redis_server, swap_run_lighthouse_tests:
+                        with swap_run_puppeteer_script:
+                            run_lighthouse_tests.main(
+                                args=[
+                                    '--mode', 'performance', '--skip_build',
+                                    '--shard', '1', '--record_screen'])