Miscellaneous fixes for docker (oppia#19923)

* Miscellaneous fixes for docker

* Update Makefile and common.py

* Fix Permission issues

* Lint fixes

* Lint fixes

.coveragerc

@@ -8,6 +8,7 @@ omit =
     *core/tests/*
     *scripts/linters/test_files/*
     *__init__.py
+    */vm_deps/*
 
 # The 'exclude_lines' is used to skip a particular clause in coverage.
 # We exclude any line with a comment of “pragma: no cover” as this is the default option

Makefile

@@ -8,6 +8,8 @@ OS_NAME := $(shell uname)
 
 FLAGS = save_datastore disable_host_checking no_auto_restart prod_env maintenance_mode source_maps
 
+sharding_instances := 3
+
 ifeq ($(OS_NAME),Darwin)
     CHROME_VERSION := $(shell /Applications/Google\ Chrome.app/Contents/MacOS/Google\ Chrome --version | awk '{print $$3}')
 else
@@ -59,7 +61,7 @@ run-offline: ## Runs the dev-server in offline mode
 start-devserver: ## Starts the development server
 	docker compose up dev-server -d
 	@printf 'Please wait while the development server starts...\n\n'
-	@while [[ $$(curl -s -o /tmp/status_code.txt -w '%{http_code}' http://localhost:8181) != "200" ]] || [[ $$(curl -s -o /tmp/status_code.txt -w '%{http_code}' http://localhost:8181/community-library) != "200" ]]; do \
+	@while [[ $$(curl -s -o .dev/status_code.txt -w '%{http_code}' http://localhost:8181) != "200" ]] || [[ $$(curl -s -o .dev/status_code.txt -w '%{http_code}' http://localhost:8181/community-library) != "200" ]]; do \
 		printf "▓"; \
 		if [[ "$(prod_env)" = 'true' ]] && [[ -n $$(docker ps -q -f status=exited -f name=webpack-compiler) ]]; then \
 			${SHELL_PREFIX} dev-server python -m scripts.generate_build_directory; \
@@ -123,22 +125,28 @@ run_tests.lint: ## Runs the linter tests
 	docker compose run --no-deps --entrypoint "/bin/sh -c 'git config --global --add safe.directory /app/oppia && python -m scripts.linters.run_lint_checks $(PYTHON_ARGS)'" dev-server || $(MAKE) stop
 
 run_tests.backend: ## Runs the backend tests
+	@echo 'Shutting down any previously started server.'
 	$(MAKE) stop
 	docker compose up datastore dev-server redis firebase -d --no-deps || $(MAKE) stop
 	@echo '------------------------------------------------------'
 	@echo '  Backend tests started....'
 	@echo '------------------------------------------------------'
-	$(SHELL_PREFIX) dev-server python -m scripts.run_backend_tests $(PYTHON_ARGS) || $(MAKE) stop
+	$(SHELL_PREFIX) dev-server sh -c "git config --global --add safe.directory /app/oppia && python -m scripts.run_backend_tests $(PYTHON_ARGS)"
 	@echo '------------------------------------------------------'
 	@echo '  Backend tests have been executed successfully....'
 	@echo '------------------------------------------------------'
 	$(MAKE) stop
 
+run_tests.check_overall_backend_test_coverage: ## Runs the check for overall backend test coverage
+	$(MAKE) start-devserver
+	$(SHELL_PREFIX) dev-server python -m scripts.check_overall_backend_test_coverage
+	$(MAKE) stop
+
 run_tests.frontend: ## Runs the frontend unit tests
 	docker compose run --no-deps --entrypoint "python -m scripts.run_frontend_tests $(PYTHON_ARGS) --skip_install" dev-server || $(MAKE) stop
 
 run_tests.typescript: ## Runs the typescript checks
-	docker compose run --no-deps --entrypoint "python -m scripts.run_typescript_checks" dev-server || $(MAKE) stop
+	docker compose run --no-deps --entrypoint "python -m scripts.typescript_checks $(PYTHON_ARGS)" dev-server || $(MAKE) stop
 
 run_tests.custom_eslint: ## Runs the custome eslint tests
 	docker compose run --no-deps --entrypoint "python -m scripts.run_custom_eslint_tests" dev-server || $(MAKE) stop
@@ -178,6 +186,7 @@ run_tests.e2e: ## Runs the e2e tests for the parsed suite
 ## CHROME_VERSION: Uses the specified version of the chrome driver.
 ## MOBILE: Run e2e test in mobile viewport.
 ## DEBUG: Runs the webdriverio test in debugging mode.
+## VIDEO_RECORDING_IS_ENABLED: Record the e2e test.
 	@echo 'Shutting down any previously started server.'
 	$(MAKE) stop
 # Adding node to the path.
@@ -187,22 +196,31 @@ run_tests.e2e: ## Runs the e2e tests for the parsed suite
 		export PATH=$(shell cd .. && pwd)/oppia_tools/node-16.13.0/bin:$(PATH); \
 	fi
 # Adding env variable for the mobile view
-	@export MOBILE=${MOBILE}
+	@export MOBILE=${MOBILE:-false}
+# Adding env variable for the video recording
+	@export VIDEO_RECORDING_IS_ENABLED=${VIDEO_RECORDING_IS_ENABLED:-0}
 # Starting the development server for the e2e tests.
 	$(MAKE) start-devserver
 	@echo '------------------------------------------------------'
 	@echo '  Starting e2e test for the suite: $(suite)'
 	@echo '------------------------------------------------------'
-	sharding_instances := 3
-	../oppia_tools/node-16.13.0/bin/npx wdio ./core/tests/wdio.conf.js --suite $(suite) $(CHROME_VERSION) --params.devMode=True --capabilities[0].maxInstances=${sharding_instances} DEBUG=${DEBUG} || $(MAKE) stop
+	../oppia_tools/node-16.13.0/bin/node ./node_modules/.bin/wdio ./core/tests/wdio.conf.js --suite $(suite) $(CHROME_VERSION) --params.devMode=True --capabilities[0].maxInstances=${sharding_instances} DEBUG=${DEBUG:-false} || $(MAKE) stop
 	@echo '------------------------------------------------------'
 	@echo '  e2e test has been executed successfully....'
 	@echo '------------------------------------------------------'
 	$(MAKE) stop
 
+run_tests.check_e2e_tests_are_captured_in_ci: ## Runs the check to ensure that all e2e tests are captured in CI
+	@echo 'Shutting down any previously started server.'
+	$(MAKE) stop
+	docker compose up dev-server -d --no-deps
+	$(SHELL_PREFIX) dev-server python -m scripts.check_e2e_tests_are_captured_in_ci
+	$(MAKE) stop
+
 run_tests.lighthouse_accessibility: ## Runs the lighthouse accessibility tests for the parsed shard
 ## Flag for Lighthouse test
 ## shard: The shard number to run the lighthouse tests
+## RECORD_SCREEN: Record the lighthouse test
 	@echo 'Shutting down any previously started server.'
 	$(MAKE) stop
 # Adding node to the path.
@@ -216,7 +234,11 @@ run_tests.lighthouse_accessibility: ## Runs the lighthouse accessibility tests f
 	@echo '-----------------------------------------------------------------------'
 	@echo '  Starting Lighthouse Accessibility tests -- shard number: $(shard)'
 	@echo '-----------------------------------------------------------------------'
-	../oppia_tools/node-16.13.0/bin/node ./core/tests/puppeteer/lighthouse_setup.js
+	@if [ "$(RECORD_SCREEN)" = "true" ]; then \
+		../oppia_tools/node-16.13.0/bin/node ./core/tests/puppeteer/lighthouse_setup.js ../lhci-puppeteer-video/video.mp4; \
+	else \
+		../oppia_tools/node-16.13.0/bin/node ./core/tests/puppeteer/lighthouse_setup.js; \
+	fi
 	../oppia_tools/node-16.13.0/bin/node ./node_modules/@lhci/cli/src/cli.js autorun --config=.lighthouserc-accessibility-${shard}.js --max-old-space-size=4096 || $(MAKE) stop
 	@echo '-----------------------------------------------------------------------'
 	@echo '  Lighthouse tests has been executed successfully....'
@@ -226,6 +248,7 @@ run_tests.lighthouse_accessibility: ## Runs the lighthouse accessibility tests f
 run_tests.lighthouse_performance: ## Runs the lighthouse performance tests for the parsed shard
 ## Flag for Lighthouse test
 ## shard: The shard number to run the lighthouse tests
+## RECORD_SCREEN: Record the lighthouse test
 	@echo 'Shutting down any previously started server.'
 	$(MAKE) stop
 # Adding node to the path.
@@ -239,7 +262,11 @@ run_tests.lighthouse_performance: ## Runs the lighthouse performance tests for t
 	@echo '-----------------------------------------------------------------------'
 	@echo '  Starting Lighthouse Performance tests -- shard number: $(shard)'
 	@echo '-----------------------------------------------------------------------'
-	../oppia_tools/node-16.13.0/bin/node ./core/tests/puppeteer/lighthouse_setup.js
+	@if [ "$(RECORD_SCREEN)" = "true" ]; then \
+		../oppia_tools/node-16.13.0/bin/node ./core/tests/puppeteer/lighthouse_setup.js ../lhci-puppeteer-video/video.mp4; \
+	else \
+		../oppia_tools/node-16.13.0/bin/node ./core/tests/puppeteer/lighthouse_setup.js; \
+	fi
 	../oppia_tools/node-16.13.0/bin/node node_modules/@lhci/cli/src/cli.js autorun --config=.lighthouserc-${shard}.js --max-old-space-size=4096 || $(MAKE) stop
 	@echo '-----------------------------------------------------------------------'
 	@echo '  Lighthouse tests has been executed successfully....'

docker-compose.yml

@@ -16,7 +16,6 @@ services:
       - NODE_OPTIONS=--max_old_space_size=4096
       - prod_env=${prod_env:-false}
     entrypoint: bash /app/oppia/docker/angular-build.entrypoint.sh
-    user: node:node
 
   webpack-compiler:
     build:

docker/Dockerfile.backend

@@ -26,18 +26,21 @@ RUN rm google-chrome.deb
 RUN pip install --upgrade pip==21.2.3
 RUN pip install pip-tools==6.6.2 setuptools==58.5.3 cmake
 
+WORKDIR /app/vm_deps
+
 # Installing google cloud sdk
-RUN wget -O gcloud-sdk.tar.gz https://dl.google.com/dl/cloudsdk/channels/rapid/downloads/google-cloud-sdk-364.0.0-linux-x86_64.tar.gz && \
+RUN wget -O /app/vm_deps/gcloud-sdk.tar.gz https://dl.google.com/dl/cloudsdk/channels/rapid/downloads/google-cloud-sdk-364.0.0-linux-x86_64.tar.gz && \
     tar -xzf gcloud-sdk.tar.gz && \
     ./google-cloud-sdk/install.sh --quiet && \
+    ./google-cloud-sdk/bin/gcloud components install beta cloud-datastore-emulator app-engine-python app-engine-python-extras && \
     rm gcloud-sdk.tar.gz
 
 WORKDIR /app/oppia
 
 ENV BUF_LINUX_FILES="buf-Linux-x86_64 protoc-gen-buf-check-lint-Linux-x86_64 protoc-gen-buf-check-breaking-Linux-x86_64" \
     PROTOC_LINUX_FILE='protoc-3.13.0-linux-x86_64.zip' \
-    BUF_DIR='/app/buf-0.29.0' \
-    PROTOC_DIR='/app/buf-0.29.0/protoc' \
+    BUF_DIR='/app/vm_deps/buf-0.29.0' \
+    PROTOC_DIR='/app/vm_deps/buf-0.29.0/protoc' \
     BUF_BASE_URL='https://github.com/bufbuild/buf/releases/download/v0.29.0/' \
     PROTOC_URL='https://github.com/protocolbuffers/protobuf/releases/download/v3.13.0' \
     PROTOC_FILES_PATH='/app/oppia/third_party/oppia-ml-proto-0.0.0'
@@ -84,7 +87,7 @@ RUN python -m scripts.install_dependencies_json_packages
 
 # Compiling the protobuf files
 RUN mkdir /app/oppia/proto_files
-RUN /app/buf-0.29.0/protoc/bin/protoc \
+RUN /app/vm_deps/buf-0.29.0/protoc/bin/protoc \
     --plugin=protoc-gen-ts=/app/oppia/node_modules/.bin/protoc-gen-ts \
     --ts_out=/app/oppia/extensions/classifiers/proto/ \
     --js_out=import_style=commonjs,binary:/app/oppia/extensions/classifiers/proto/ \

docker/Dockerfile.frontend

@@ -23,4 +23,3 @@ RUN yarn install --pure-lockfile
 # Set the ownership of the Yarn cache directory to the 'node' user and group.
 # This action guarantees that the yarn cache is owned by the node user.
 RUN chown -R node:node /home/node/.yarn-cache
-USER node:node

docker/datastore.entrypoint.sh

@@ -13,7 +13,7 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 
-datastore_cmd="/google-cloud-sdk/bin/gcloud beta emulators datastore start \
+datastore_cmd="/app/vm_deps/google-cloud-sdk/bin/gcloud beta emulators datastore start \
 --project=dev-project-id \
 --data-dir=/app/cloud_datastore_emulator_cache \
 --host-port=0.0.0.0:8089 \

docker/dev-server.entrypoint.sh

@@ -14,7 +14,7 @@
 # limitations under the License.
 
 build_cmd="python -m scripts.build"
-dev_appserver_cmd="/google-cloud-sdk/bin/dev_appserver.py \
+dev_appserver_cmd="/app/vm_deps/google-cloud-sdk/bin/dev_appserver.py \
 --runtime=python38 \
 --host=0.0.0.0 \
 --port=8181 \

scripts/common.py

@@ -86,9 +86,14 @@
 THIRD_PARTY_DIR = os.path.join(CURR_DIR, 'third_party')
 THIRD_PARTY_PYTHON_LIBS_DIR = os.path.join(THIRD_PARTY_DIR, 'python_libs')
 GOOGLE_CLOUD_SDK_HOME = (
-    '/google-cloud-sdk' if feconf.OPPIA_IS_DOCKERIZED else os.path.join(
-        OPPIA_TOOLS_DIR_ABS_PATH, 'google-cloud-sdk-364.0.0', 'google-cloud-sdk'
-    ))
+    '/app/vm_deps/google-cloud-sdk'
+    if feconf.OPPIA_IS_DOCKERIZED
+    else os.path.join(
+        OPPIA_TOOLS_DIR_ABS_PATH,
+        'google-cloud-sdk-364.0.0',
+        'google-cloud-sdk'
+    )
+)
 GOOGLE_APP_ENGINE_SDK_HOME = os.path.join(
     GOOGLE_CLOUD_SDK_HOME, 'platform', 'google_appengine')
 GOOGLE_CLOUD_SDK_BIN = os.path.join(GOOGLE_CLOUD_SDK_HOME, 'bin')

scripts/common_test.py

@@ -1257,67 +1257,70 @@ def test_modify_constants(self) -> None:
             common, 'CONSTANTS_FILE_PATH', mock_constants_path)
         feconf_path_swap = self.swap(common, 'FECONF_PATH', mock_feconf_path)
 
-        def mock_check_output(
-            unused_cmd_tokens: List[str], encoding: str = 'utf-8'  # pylint: disable=unused-argument
-        ) -> str:
-            return 'test'
-        check_output_swap = self.swap(
-            subprocess, 'check_output', mock_check_output
-        )
+        with self.swap(feconf, 'OPPIA_IS_DOCKERIZED', False):
+            def mock_check_output(
+                unused_cmd_tokens: List[str], encoding: str = 'utf-8'  # pylint: disable=unused-argument
+            ) -> str:
+                return 'test'
+            check_output_swap = self.swap(
+                subprocess, 'check_output', mock_check_output
+            )
 
-        constants_temp_file = tempfile.NamedTemporaryFile()
-        # Here MyPy assumes that the 'name' attribute is read-only. In order to
-        # silence the MyPy complaints `setattr` is used to set the attribute.
-        setattr(
-            constants_temp_file, 'name', mock_constants_path)
-        with utils.open_file(mock_constants_path, 'w') as tmp:
-            tmp.write('export = {\n')
-            tmp.write('  "DEV_MODE": true,\n')
-            tmp.write('  "EMULATOR_MODE": false,\n')
-            tmp.write('  "BRANCH_NAME": "",\n')
-            tmp.write('  "SHORT_COMMIT_HASH": ""\n')
-            tmp.write('};')
+            constants_temp_file = tempfile.NamedTemporaryFile()
+            # Here MyPy assumes that the 'name' attribute is read-only.
+            # In order to silence the MyPy complaints `setattr` is used
+            # to set the attribute.
+            setattr(
+                constants_temp_file, 'name', mock_constants_path)
+            with utils.open_file(mock_constants_path, 'w') as tmp:
+                tmp.write('export = {\n')
+                tmp.write('  "DEV_MODE": true,\n')
+                tmp.write('  "EMULATOR_MODE": false,\n')
+                tmp.write('  "BRANCH_NAME": "",\n')
+                tmp.write('  "SHORT_COMMIT_HASH": ""\n')
+                tmp.write('};')
 
-        feconf_temp_file = tempfile.NamedTemporaryFile()
-        # Here MyPy assumes that the 'name' attribute is read-only. In order to
-        # silence the MyPy complaints `setattr` is used to set the attribute.
-        setattr(feconf_temp_file, 'name', mock_feconf_path)
-        with utils.open_file(mock_feconf_path, 'w') as tmp:
-            tmp.write(u'ENABLE_MAINTENANCE_MODE = False')
+            feconf_temp_file = tempfile.NamedTemporaryFile()
+            # Here MyPy assumes that the 'name' attribute is read-only.
+            # In order to silence the MyPy complaints `setattr` is used
+            # to set the attribute.
+            setattr(feconf_temp_file, 'name', mock_feconf_path)
+            with utils.open_file(mock_feconf_path, 'w') as tmp:
+                tmp.write(u'ENABLE_MAINTENANCE_MODE = False')
 
-        with constants_path_swap, feconf_path_swap, check_output_swap:
-            common.modify_constants(prod_env=True, maintenance_mode=False)
-            with utils.open_file(
-                mock_constants_path, 'r') as constants_file:
-                self.assertEqual(
-                    constants_file.read(),
-                    'export = {\n'
-                    '  "DEV_MODE": false,\n'
-                    '  "EMULATOR_MODE": true,\n'
-                    '  "BRANCH_NAME": "test",\n'
-                    '  "SHORT_COMMIT_HASH": "test"\n'
-                    '};')
-            with utils.open_file(mock_feconf_path, 'r') as feconf_file:
-                self.assertEqual(
-                    feconf_file.read(), 'ENABLE_MAINTENANCE_MODE = False')
+            with constants_path_swap, feconf_path_swap, check_output_swap:
+                common.modify_constants(prod_env=True, maintenance_mode=False)
+                with utils.open_file(
+                    mock_constants_path, 'r') as constants_file:
+                    self.assertEqual(
+                        constants_file.read(),
+                        'export = {\n'
+                        '  "DEV_MODE": false,\n'
+                        '  "EMULATOR_MODE": true,\n'
+                        '  "BRANCH_NAME": "test",\n'
+                        '  "SHORT_COMMIT_HASH": "test"\n'
+                        '};')
+                with utils.open_file(mock_feconf_path, 'r') as feconf_file:
+                    self.assertEqual(
+                        feconf_file.read(), 'ENABLE_MAINTENANCE_MODE = False')
 
-            common.modify_constants(prod_env=False, maintenance_mode=True)
-            with utils.open_file(
-                mock_constants_path, 'r') as constants_file:
-                self.assertEqual(
-                    constants_file.read(),
-                    'export = {\n'
-                    '  "DEV_MODE": true,\n'
-                    '  "EMULATOR_MODE": true,\n'
-                    '  "BRANCH_NAME": "test",\n'
-                    '  "SHORT_COMMIT_HASH": "test"\n'
-                    '};')
-            with utils.open_file(mock_feconf_path, 'r') as feconf_file:
-                self.assertEqual(
-                    feconf_file.read(), 'ENABLE_MAINTENANCE_MODE = True')
+                common.modify_constants(prod_env=False, maintenance_mode=True)
+                with utils.open_file(
+                    mock_constants_path, 'r') as constants_file:
+                    self.assertEqual(
+                        constants_file.read(),
+                        'export = {\n'
+                        '  "DEV_MODE": true,\n'
+                        '  "EMULATOR_MODE": true,\n'
+                        '  "BRANCH_NAME": "test",\n'
+                        '  "SHORT_COMMIT_HASH": "test"\n'
+                        '};')
+                with utils.open_file(mock_feconf_path, 'r') as feconf_file:
+                    self.assertEqual(
+                        feconf_file.read(), 'ENABLE_MAINTENANCE_MODE = True')
 
-        constants_temp_file.close()
-        feconf_temp_file.close()
+            constants_temp_file.close()
+            feconf_temp_file.close()
 
         # Clean up spare files.
         os.remove(mock_constants_path)
@@ -1350,21 +1353,22 @@ def test_set_constants_to_default(self) -> None:
         with utils.open_file(mock_feconf_path, 'w') as tmp:
             tmp.write(u'ENABLE_MAINTENANCE_MODE = True')
         self.contextManager.__exit__(None, None, None)
-        with constants_path_swap, feconf_path_swap:
-            common.set_constants_to_default()
-            with utils.open_file(
-                mock_constants_path, 'r') as constants_file:
-                self.assertEqual(
-                    constants_file.read(),
-                    'export = {\n'
-                    '  "DEV_MODE": true,\n'
-                    '  "EMULATOR_MODE": true,\n'
-                    '  "BRANCH_NAME": "",\n'
-                    '  "SHORT_COMMIT_HASH": ""\n'
-                    '};')
-            with utils.open_file(mock_feconf_path, 'r') as feconf_file:
-                self.assertEqual(
-                    feconf_file.read(), 'ENABLE_MAINTENANCE_MODE = False')
+        with self.swap(feconf, 'OPPIA_IS_DOCKERIZED', False):
+            with constants_path_swap, feconf_path_swap:
+                common.set_constants_to_default()
+                with utils.open_file(
+                    mock_constants_path, 'r') as constants_file:
+                    self.assertEqual(
+                        constants_file.read(),
+                        'export = {\n'
+                        '  "DEV_MODE": true,\n'
+                        '  "EMULATOR_MODE": true,\n'
+                        '  "BRANCH_NAME": "",\n'
+                        '  "SHORT_COMMIT_HASH": ""\n'
+                        '};')
+                with utils.open_file(mock_feconf_path, 'r') as feconf_file:
+                    self.assertEqual(
+                        feconf_file.read(), 'ENABLE_MAINTENANCE_MODE = False')
         constants_temp_file.close()
         feconf_temp_file.close()