From c6aaacdbf17eb8e6d58f6d6cd8ccb4d511d211f0 Mon Sep 17 00:00:00 2001 From: Antonio Cuni Date: Wed, 27 Sep 2023 15:02:49 +0000 Subject: [PATCH] Re-enable CI tests (#1760) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This re-enables CI tests on every PR 🎉. This uses make test-integration, which runs tests sequentially. In theory, we also have test-integration-parallel but it seems to be very flaky: many tests randomly timeout. I didn't spend too much time investigating this, it's probably worth its own investigation in a separate PR, but for now it's important to re-enable CI tests, even if they are a bit slower. --- .github/workflows/test.yml | 83 +++++++++++++++++++ .github/workflows/test_report.yml | 16 ++++ Makefile | 42 ++++------ pyscriptjs/environment.yml => environment.yml | 0 pyscript.core/tests/integration/support.py | 7 +- .../tests/integration/test_01_basic.py | 42 +++++----- .../tests/integration/test_02_display.py | 2 +- .../tests/integration/test_py_config.py | 1 - 8 files changed, 141 insertions(+), 52 deletions(-) create mode 100644 .github/workflows/test.yml create mode 100644 .github/workflows/test_report.yml rename pyscriptjs/environment.yml => environment.yml (100%) diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml new file mode 100644 index 00000000..cbe1ff49 --- /dev/null +++ b/.github/workflows/test.yml @@ -0,0 +1,83 @@ +name: "[CI] Test" + +on: + push: # Only run on merges into main that modify certain files + branches: + - main + paths: + - pyscript.core/** + - .github/workflows/test.yml + + pull_request: # Only run on merges into main that modify certain files + branches: + - main + paths: + - pyscript.core/** + - .github/workflows/test.yml + workflow_dispatch: + +jobs: + BuildAndTest: + runs-on: ubuntu-latest-8core + env: + MINICONDA_PYTHON_VERSION: py38 + MINICONDA_VERSION: 4.11.0 + steps: + - name: Checkout + uses: actions/checkout@v4 + with: + fetch-depth: 3 + + # display a git log: when you run CI on PRs, github automatically + # merges the PR into main and run the CI on that commit. The idea + # here is to show enough of git log to understand what is the + # actual commit (in the PR) that we are using. See also + # 'fetch-depth: 3' above. + - name: git log + run: git log --graph -3 + + - name: Install node + uses: actions/setup-node@v3 + with: + node-version: 20.x + + - name: Cache node modules + uses: actions/cache@v3 + env: + cache-name: cache-node-modules + with: + # npm cache files are stored in `~/.npm` on Linux/macOS + path: ~/.npm + key: ${{ runner.os }}-build-${{ env.cache-name }}-${{ hashFiles('**/package-lock.json') }} + restore-keys: | + ${{ runner.os }}-build-${{ env.cache-name }}- + ${{ runner.os }}-build- + ${{ runner.os }}- + + - name: setup Miniconda + uses: conda-incubator/setup-miniconda@v2 + + - name: Setup Environment + run: make setup + + - name: Build + run: make build + + - name: Integration Tests + #run: make test-integration-parallel + run: make test-integration + + - uses: actions/upload-artifact@v3 + with: + name: pyscript + path: | + pyscript.core/dist/ + if-no-files-found: error + retention-days: 7 + + - uses: actions/upload-artifact@v3 + if: success() || failure() + with: + name: test_results + path: test_results/ + if-no-files-found: error diff --git a/.github/workflows/test_report.yml b/.github/workflows/test_report.yml new file mode 100644 index 00000000..6debdbdc --- /dev/null +++ b/.github/workflows/test_report.yml @@ -0,0 +1,16 @@ +name: Test Report +on: + workflow_run: + workflows: ['\[CI\] Test'] + types: + - completed +jobs: + report: + runs-on: ubuntu-latest-8core + steps: + - uses: dorny/test-reporter@v1.6.0 + with: + artifact: test_results + name: Test reports + path: "*.xml" + reporter: java-junit diff --git a/Makefile b/Makefile index aaea8212..fde0a05d 100644 --- a/Makefile +++ b/Makefile @@ -7,12 +7,13 @@ examples ?= ../$(base_dir)/examples app_dir ?= $(shell git rev-parse --show-prefix) CONDA_EXE := conda -CONDA_ENV ?= $(base_dir)/pyscriptjs/env +CONDA_ENV ?= $(base_dir)/env env := $(CONDA_ENV) conda_run := $(CONDA_EXE) run -p $(env) PYTEST_EXE := $(CONDA_ENV)/bin/pytest -GOOD_NODE_VER := 14 -GOOD_NPM_VER := 6 + +MIN_NODE_VER := 14 +MIN_NPM_VER := 6 NODE_VER := $(shell node -v | cut -d. -f1 | sed 's/^v\(.*\)/\1/') NPM_VER := $(shell npm -v | cut -d. -f1) @@ -22,20 +23,21 @@ else SED_I_ARG := -i endif -GOOD_NODE := $(shell if [ $(NODE_VER) -ge $(GOOD_NODE_VER) ]; then echo true; else echo false; fi) -GOOD_NPM := $(shell if [ $(NPM_VER) -ge $(GOOD_NPM_VER) ]; then echo true; else echo false; fi) - .PHONY: check-node check-node: - @echo Build requires Node $(GOOD_NODE_VER).x or higher: $(NODE_VER) detected && $(GOOD_NODE) + @if [ $(NODE_VER) -lt $(MIN_NODE_VER) ]; then \ + echo "Build requires Node $(MIN_NODE_VER).x or higher: $(NODE_VER) detected"; \ + false; \ + fi .PHONY: check-npm check-npm: - @echo Build requires npm $(GOOD_NPM_VER).x or higher: $(NPM_VER) detected && $(GOOD_NPM) + @if [ $(NPM_VER) -lt $(MIN_NPM_VER) ]; then \ + echo "Build requires Node $(MIN_NPM_VER).x or higher: $(NPM_VER) detected"; \ + false; \ + fi -setup: - make check-node - make check-npm +setup: check-node check-npm cd pyscript.core && npm install && cd .. $(CONDA_EXE) env $(shell [ -d $(env) ] && echo update || echo create) -p $(env) --file environment.yml $(conda_run) playwright install @@ -53,13 +55,10 @@ shell: @echo 'conda activate $(env)' dev: - npm run dev + cd pyscript.core && npm run dev build: - npm run build - -build-fast: - node esbuild.mjs + cd pyscript.core && npm run build # use the following rule to do all the checks done by precommit: in # particular, use this if you want to run eslint. @@ -85,27 +84,22 @@ run-examples: setup build examples npm install make dev -test: - cd pyscript.core && npm run build && cp -R dist ../pyscriptjs/build/ - make test-integration - make test-examples - # run all integration tests *including examples* sequentially # TODO: (fpliger) The cd pyscript.core before running the tests shouldn't be needed but for # but for some reason it seems to bother pytest tmppaths (or test cache?). Unclear. test-integration: mkdir -p test_results - cd pyscript.core && $(PYTEST_EXE) -vv $(ARGS) tests/integration/ --log-cli-level=warning --junitxml=../integration.xml + $(PYTEST_EXE) -vv $(ARGS) pyscript.core/tests/integration/ --log-cli-level=warning --junitxml=test_results/integration.xml # run all integration tests *except examples* in parallel (examples use too much memory) test-integration-parallel: mkdir -p test_results - $(PYTEST_EXE) --numprocesses auto -vv $(ARGS) pyscript/tests/integration/ --log-cli-level=warning --junitxml=test_results/integration.xml -k 'not zz_examples' + $(PYTEST_EXE) --numprocesses auto -vv $(ARGS) pyscript.core/tests/integration/ --log-cli-level=warning --junitxml=test_results/integration.xml # run integration tests on only examples sequentially (to avoid running out of memory) test-examples: mkdir -p test_results - $(PYTEST_EXE) -vv $(ARGS) pyscript/tests/integration/ --log-cli-level=warning --junitxml=test_results/integration.xml -k 'zz_examples' + $(PYTEST_EXE) -vv $(ARGS) pyscript.core/tests/integration/ --log-cli-level=warning --junitxml=test_results/integration.xml -k 'zz_examples' test-py: @echo "Tests from $(src_dir)" diff --git a/pyscriptjs/environment.yml b/environment.yml similarity index 100% rename from pyscriptjs/environment.yml rename to environment.yml diff --git a/pyscript.core/tests/integration/support.py b/pyscript.core/tests/integration/support.py index 22812650..dd8d2235 100644 --- a/pyscript.core/tests/integration/support.py +++ b/pyscript.core/tests/integration/support.py @@ -169,7 +169,7 @@ class PyScriptTest: creates an HTML page to run the specified snippet. """ - DEFAULT_TIMEOUT = 20000 + DEFAULT_TIMEOUT = 30 * 1000 @pytest.fixture() def init(self, request, tmpdir, logger, page, execution_thread): @@ -240,10 +240,7 @@ class PyScriptTest: def init_page(self, page): self.page = page - - # set default timeout to 60000 millliseconds from 30000 page.set_default_timeout(self.DEFAULT_TIMEOUT) - self.console = ConsoleMessageCollection(self.logger) self._js_errors = [] self._py_errors = [] @@ -424,7 +421,7 @@ class PyScriptTest: return text in self.console.all.lines if timeout is None: - timeout = 10 * 1000 + timeout = self.DEFAULT_TIMEOUT # NOTE: we cannot use playwright's own page.expect_console_message(), # because if you call it AFTER the text has already been emitted, it # waits forever. Instead, we have to use our own custom logic. diff --git a/pyscript.core/tests/integration/test_01_basic.py b/pyscript.core/tests/integration/test_01_basic.py index 6bb306cc..3696b4f2 100644 --- a/pyscript.core/tests/integration/test_01_basic.py +++ b/pyscript.core/tests/integration/test_01_basic.py @@ -11,26 +11,22 @@ class TestBasic(PyScriptTest): """ + """ + ) + assert self.console.log.lines == ["hello from script py"] + def test_py_script_hello(self): + self.pyscript_run( + """ import js - js.console.log('2. hello from py-script') + js.console.log('hello from py-script') """ ) - if self.execution_thread == "main": - # in main, the order of execution is guaranteed - assert self.console.log.lines == [ - "1. hello from script py", - "2. hello from py-script", - ] - else: - # in workers, each tag is executed by its own worker, so they can - # come out of order - lines = sorted(self.console.log.lines) - assert lines == ["1. hello from script py", "2. hello from py-script"] + assert self.console.log.lines == ["hello from py-script"] def test_execution_thread(self): self.pyscript_run( @@ -47,20 +43,25 @@ class TestBasic(PyScriptTest): in_worker = str(in_worker).lower() assert self.console.log.lines[-1] == f"worker? {in_worker}" - # TODO: if there's no py-script there are surely no plugins neither - # this test must be discussed or rewritten to make sense now - @pytest.mark.skip(reason="NEXT: No banner and should also add a WARNING about CORS") + @skip_worker('NEXT: it should show a nice error on the page') def test_no_cors_headers(self): self.disable_cors_headers() self.pyscript_run( """ - + """, wait_for_pyscript=False, ) assert self.headers == {} - if self.execution_thread == "worker": + if self.execution_thread == "main": + self.wait_for_pyscript() + assert self.console.log.lines == ['hello'] + self.assert_no_banners() + else: + # XXX adapt and fix the test expected_alert_banner_msg = ( '(PY1000): When execution_thread is "worker", the site must be cross origin ' "isolated, but crossOriginIsolated is false. To be cross origin isolated, " @@ -71,8 +72,7 @@ class TestBasic(PyScriptTest): ) alert_banner = self.page.wait_for_selector(".alert-banner") assert expected_alert_banner_msg in alert_banner.inner_text() - else: - self.assert_no_banners() + def test_print(self): self.pyscript_run( diff --git a/pyscript.core/tests/integration/test_02_display.py b/pyscript.core/tests/integration/test_02_display.py index 9fc227e5..09e4a170 100644 --- a/pyscript.core/tests/integration/test_02_display.py +++ b/pyscript.core/tests/integration/test_02_display.py @@ -457,7 +457,7 @@ class TestDisplay(PyScriptTest): img = Image.new("RGB", (4, 4), color=(0, 0, 0)) display(img, target='img-target', append=False) - """ + """, ) img_src = self.page.locator("img").get_attribute("src") diff --git a/pyscript.core/tests/integration/test_py_config.py b/pyscript.core/tests/integration/test_py_config.py index 0a868115..d6576485 100644 --- a/pyscript.core/tests/integration/test_py_config.py +++ b/pyscript.core/tests/integration/test_py_config.py @@ -67,7 +67,6 @@ class TestConfig(PyScriptTest): ) assert self.console.log.lines[-1] == "config name: app with external config" - @pytest.mark.skip("NEXT: We need to restore the banner.") def test_invalid_json_config(self): # we need wait_for_pyscript=False because we bail out very soon, # before being able to write 'PyScript page fully initialized'