From a4ec5992f30a7708bbfa7591a5c12cc9f83471cd Mon Sep 17 00:00:00 2001 From: nebay-abraha Date: Thu, 16 Apr 2026 10:08:40 +0100 Subject: [PATCH 01/14] - migrate pytest config to pyproject and align test dependencies - bump pytest and pin setuptools - add RSpace bootstrapping and python playwright api key generation - update development and test run guidance - python version baseline and clearer unit vs integration instructions - make test more robust - run integration tests against rspace-docker container - update barcode data class to align with rspace-java-client - add sample_post tests --- .env.example | 5 +- .github/scripts/get_rspace_api_key.py | 98 +++++++++++++++ .github/workflows/codeql-and-tests.yml | 152 +++++++++++++++++++++--- DEVELOPING.md | 25 ++-- pyproject.toml | 9 +- pytest.ini | 2 - rspace_client/inv/inv.py | 31 +++-- rspace_client/tests/elnapi_test.py | 72 ++++++----- rspace_client/tests/inv_lom_test.py | 3 + rspace_client/tests/invapi_test.py | 28 +++-- rspace_client/tests/sample_post_test.py | 59 +++++++++ 11 files changed, 406 insertions(+), 78 deletions(-) create mode 100644 .github/scripts/get_rspace_api_key.py delete mode 100644 pytest.ini create mode 100644 rspace_client/tests/sample_post_test.py diff --git a/.env.example b/.env.example index 8469ca8..d0a1977 100644 --- a/.env.example +++ b/.env.example @@ -1,2 +1,5 @@ RSPACE_URL= -RSPACE_API_KEY= \ No newline at end of file +RSPACE_API_KEY= + +RSPACE_USERNAME= +RSPACE_PASSWORD= \ No newline at end of file diff --git a/.github/scripts/get_rspace_api_key.py b/.github/scripts/get_rspace_api_key.py new file mode 100644 index 0000000..9351359 --- /dev/null +++ b/.github/scripts/get_rspace_api_key.py @@ -0,0 +1,98 @@ +import os +import re +import sys +from playwright.sync_api import sync_playwright, expect +from dotenv import load_dotenv + +load_dotenv() + +RSPACE_URL = os.getenv("RSPACE_URL") +RSPACE_USERNAME = os.getenv("RSPACE_USERNAME") +RSPACE_PASSWORD = os.getenv("RSPACE_PASSWORD") + + +def main(): + if not RSPACE_URL or not RSPACE_USERNAME or not RSPACE_PASSWORD: + raise RuntimeError( + "Missing required environment variables: RSPACE_URL, RSPACE_USERNAME, RSPACE_PASSWORD" + ) + + with sync_playwright() as p: + browser = p.chromium.launch(headless=True) + context = browser.new_context() + page = context.new_page() + + print("Step 1: Navigating to RSpace...", file=sys.stderr) + page.goto(RSPACE_URL, wait_until="networkidle") + + print("Step 2: Logging in...", file=sys.stderr) + page.get_by_role("textbox", name="User").fill(RSPACE_USERNAME) + page.get_by_role("textbox", name="Password").fill(RSPACE_PASSWORD) + page.get_by_role("button", name="Log in").click() + + # Wait for login to complete and redirect + page.wait_for_load_state("networkidle") + print("Step 3: Login successful, navigating to My RSpace...", file=sys.stderr) + + # Step 2: Navigate to Profile → Manage API Key + page.get_by_role("link", name="My RSpace").click() + page.wait_for_load_state("networkidle") + + print("Step 4: Looking for Generate/Regenerate key button...", file=sys.stderr) + # Wait for the button to appear (either "Generate key" or "Regenerate key") + page.wait_for_selector("a#apiKeyRegenerateBtn", timeout=10000) + page.click("a#apiKeyRegenerateBtn") + + print("Step 5: Confirming password...", file=sys.stderr) + dialog = page.get_by_role("dialog", name="Confirm password") + dialog.wait_for() + dialog.get_by_role("textbox", name="Please confirm your password").fill(RSPACE_PASSWORD) + dialog.get_by_role("button", name="OK").click() + dialog.wait_for(state="detached") + + + print("Step 6: Waiting for key to be displayed...", file=sys.stderr) + page.wait_for_load_state("networkidle") + + # Wait for the key to appear in the page + page.wait_for_selector("#apiKeyInfo") + + # Extract the key from the displayed text + # Format: "Key: {32-char-string}" + key_locator = page.locator("div.api-menu__key") + if key_locator.count() == 0: + raise RuntimeError("Selector 'div.api-menu__key' not found on page — page structure may have changed") + + info_text = key_locator.inner_text() + print(f"Key element text length: {len(info_text)}", file=sys.stderr) + + match = re.search(r"Key:\s*([A-Za-z0-9]{32})", info_text) + if not match: + raise RuntimeError( + f"API key regex did not match — text length {len(info_text)}, starts with: {repr(info_text[:20])}" + ) + + api_key = match.group(1) + print("Successfully extracted API key", file=sys.stderr) + + # Write to GITHUB_OUTPUT so the key never touches stdout/stderr. + # Writing to GITHUB_OUTPUT is the standard GitHub Actions mechanism for passing + # step outputs; the file is ephemeral and runner-scoped. + github_output = os.getenv("GITHUB_OUTPUT") + if github_output: + with open(github_output, "a") as f: + f.write(f"rspace_api_key={api_key}\n") + else: + raise RuntimeError("GITHUB_OUTPUT is not set; refusing to emit API key to stdout.") + + browser.close() + + + +if __name__ == "__main__": + try: + main() + except Exception as exc: + print(f"Error generating API key: {exc}", file=sys.stderr)c + sys.exit(1) + \ No newline at end of file diff --git a/.github/workflows/codeql-and-tests.yml b/.github/workflows/codeql-and-tests.yml index 77ae1db..9b8d063 100644 --- a/.github/workflows/codeql-and-tests.yml +++ b/.github/workflows/codeql-and-tests.yml @@ -1,17 +1,21 @@ -name: CodeQL and Unit Test +name: codeQL and Test on: + push: + branches: [ master ] pull_request: branches: [ master ] workflow_dispatch: # allow manual trigger +permissions: + contents: read + jobs: analyze: name: CodeQL Analyze - runs-on: ubuntu-latest + runs-on: ubuntu-24.04 + timeout-minutes: 20 permissions: - actions: read - contents: read security-events: write strategy: fail-fast: false @@ -20,35 +24,147 @@ jobs: steps: - name: Checkout repository - uses: actions/checkout@v3 + uses: actions/checkout@v4 - name: Initialize CodeQL - uses: github/codeql-action/init@v3 + uses: github/codeql-action/init@v4 with: languages: ${{ matrix.language }} - name: Autobuild - uses: github/codeql-action/autobuild@v3 + uses: github/codeql-action/autobuild@v4 - name: Perform CodeQL Analysis - uses: github/codeql-action/analyze@v3 + uses: github/codeql-action/analyze@v4 test: - name: Unit Test - runs-on: ubuntu-latest + name: unit test + runs-on: ubuntu-24.04 + timeout-minutes: 30 needs: analyze + permissions: + contents: read + strategy: + matrix: + python-version: ["3.9", "3.10", "3.11", "3.12"] steps: - name: Checkout repository - uses: actions/checkout@v3 + uses: actions/checkout@v4 - - name: Set up Poetry - run: | - curl -sSL https://install.python-poetry.org | python3 - - echo 'export PATH="$HOME/.local/bin:$PATH"' >> $GITHUB_ENV + - name: Set up Python ${{ matrix.python-version }} + uses: actions/setup-python@v5 + with: + python-version: ${{ matrix.python-version }} + + - name: Install Poetry + uses: snok/install-poetry@v1 + with: + version: 1.8.4 + virtualenvs-create: true + virtualenvs-in-project: true + + - name: Install dependencies + run: poetry install --no-interaction + + - name: Run unit tests + run: poetry run pytest -m "not integration" + + integration-test: + name: integration test + runs-on: ubuntu-24.04 + timeout-minutes: 45 + needs: test + permissions: + contents: read + + steps: + - name: Checkout repository + uses: actions/checkout@v4 + + - name: Set up Python 3.12 + uses: actions/setup-python@v5 + with: + python-version: "3.12" + + - name: Install Poetry + uses: snok/install-poetry@v1 + with: + version: 1.8.4 + virtualenvs-create: true + virtualenvs-in-project: true - name: Install dependencies - run: poetry install + run: poetry install --no-interaction + + - name: Clone rspace-docker + run: git clone https://github.com/rspace-os/rspace-docker.git /tmp/rspace-docker + + - name: Download latest RSpace WAR + run: | + set -euo pipefail + + release_json=$(curl -fsSL https://api.github.com/repos/rspace-os/rspace-web/releases/latest) + latest_tag=$(echo "$release_json" | jq -r '.tag_name') + war_url=$(echo "$release_json" | jq -r '.assets[]? | select(.name | test("^researchspace-.*\\.war$")) | .browser_download_url' | head -n1) + + if [ -z "$latest_tag" ] || [ "$latest_tag" = "null" ]; then + echo "Unable to determine latest release tag for rspace-web" + exit 1 + fi + + if [ -z "$war_url" ] || [ "$war_url" = "null" ]; then + war_url="https://github.com/rspace-os/rspace-web/releases/download/${latest_tag}/researchspace-${latest_tag}.war" + fi + + echo "Latest RSpace tag: $latest_tag" + curl -fsSL "$war_url" -o /tmp/rspace-docker/rspace.war + + - name: Free up disk space + run: | + df -h + sudo rm -rf /usr/share/dotnet /usr/local/lib/android /opt/ghc /opt/hostedtoolcache/CodeQL || true + docker image prune -af || true + df -h + + - name: Start RSpace + working-directory: /tmp/rspace-docker + run: docker compose up -d + + - name: Wait for RSpace to be ready + timeout-minutes: 10 + run: | + set -euo pipefail + + echo "Waiting for RSpace on http://localhost:8080 ..." + for i in {1..40}; do + if timeout 2 bash -c "cat < /dev/null > /dev/tcp/localhost/8080" 2>/dev/null; then + echo "RSpace is up" + exit 0 + fi + echo " attempt $i/40..." + sleep 15 + done + echo "RSpace failed to start" + exit 1 + + - name: Install Playwright Browsers + run: poetry run python -m playwright install chromium --with-deps + + - name: Generate RSpace API Key + id: generate_key + run: poetry run python .github/scripts/get_rspace_api_key.py + env: + RSPACE_URL: http://localhost:8080 + RSPACE_USERNAME: ${{ secrets.RSPACE_USERNAME }} + RSPACE_PASSWORD: ${{ secrets.RSPACE_PASSWORD }} + + - name: Mask API key + run: echo "::add-mask::${{ steps.generate_key.outputs.rspace_api_key }}" - - name: Run Unit Test - run: poetry run pytest rspace_client/tests + - name: Run integration tests + timeout-minutes: 20 + env: + RSPACE_URL: http://localhost:8080 + RSPACE_API_KEY: ${{ steps.generate_key.outputs.rspace_api_key }} + run: poetry run pytest -m integration -v \ No newline at end of file diff --git a/DEVELOPING.md b/DEVELOPING.md index 1152ce0..062200f 100644 --- a/DEVELOPING.md +++ b/DEVELOPING.md @@ -1,6 +1,6 @@ ## Development -Python 3.7 or later is required. We aim to support only active versions of Python. +Python 3.9 or later is required. We aim to support only active versions of Python. ### Setup @@ -21,23 +21,30 @@ to install all project dependencies into your virtual environment. ### Running tests -Tests are a mixture of plain unit tests and integration tests making calls to an RSpace server. -To run all tests, set these environment variables,replacing with your own values +Tests are a mixture of plain unit tests and integration tests that make calls to a live RSpace server. + +#### Unit tests only ``` -bash> export RSPACE_URL=https:/ -bash> export RSPACE_API_KEY=abcdefgh... +poetry run pytest -m "not integration" ``` -If these aren't set, integration tests will be skipped. +#### Integration tests + +Integration tests require credentials for a live RSpace instance. Create a `.env` file in the project root: + +``` +RSPACE_URL=https:// +RSPACE_API_KEY= +``` -Tests can be invoked: +Then run: ``` -poetry run pytest rspace_client/tests +poetry run pytest -m integration ``` -They should be run with a new RSpace account that does not belong to any groups. +Integration tests should be run with a new RSpace account that does not belong to any groups. ### Writing Tests diff --git a/pyproject.toml b/pyproject.toml index 0f848a8..f7325aa 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -30,8 +30,15 @@ setuptools = "<82" [tool.poetry.group.dev.dependencies] python-dotenv = "^1.1.1" black = "^21.6b0" -pytest = "^6.2.4" +pytest = "^8.0.0" +playwright = "^1.58.0" [build-system] requires = ["poetry-core>=1.0.0"] build-backend = "poetry.core.masonry.api" + +[tool.pytest.ini_options] +testpaths = ["rspace_client/tests"] +markers = [ + "integration: marks tests as integration tests requiring a live RSpace server (deselect with '-m not integration')", +] diff --git a/pytest.ini b/pytest.ini deleted file mode 100644 index de19c9f..0000000 --- a/pytest.ini +++ /dev/null @@ -1,2 +0,0 @@ -[pytest] -testpaths = tests \ No newline at end of file diff --git a/rspace_client/inv/inv.py b/rspace_client/inv/inv.py index 6267ec9..55bc652 100644 --- a/rspace_client/inv/inv.py +++ b/rspace_client/inv/inv.py @@ -8,7 +8,7 @@ import requests import pprint import requests -from typing import Optional, Sequence, Union, List, TypedDict, BinaryIO +from typing import Optional, Sequence, Union, List, TypedDict, BinaryIO, ClassVar from rspace_client.client_base import ClientBase, Pagination from rspace_client.inv import quantity_unit as qu @@ -33,19 +33,22 @@ class Tag(TypedDict): @dataclass class Barcode: - data: str - format: BarcodeFormat + data: Optional[str] = None + format: Optional[BarcodeFormat] = None description: str = "" - newBarcodeRequest: bool = True - id: Optional[str] = "" + new_barcode_request: bool = True + id: Optional[str] = None def to_dict(self): - return{ - "data": self.data, - "format": self.format.value, + result = { "description": self.description, - "newBarcodeRequest": self.newBarcodeRequest + "newBarcodeRequest": self.new_barcode_request, } + if self.data is not None: + result["data"] = self.data + if self.format is not None: + result["format"] = self.format.value + return result class FillingStrategy(Enum): @@ -815,7 +818,8 @@ def __init__( subsample_count: int = None, total_quantity: Quantity = None, attachments=None, - barcodes: Optional[List[Barcode]] = None + barcodes: Optional[List[Barcode]] = None, + location: "TargetLocation" = None, ): super().__init__(name, "SAMPLE", tags, description, extra_fields) ## converts arguments into JSON POST syntax @@ -834,10 +838,11 @@ def __init__( self.data["templateId"] = sample_template_id if fields is not None: self.data["fields"] = fields + if location is not None: + self.data.update(location.data) ## fail early - if attachments is not None: - if not isinstance(attachments, list): - raise ValueError("attachments must be a list of open files") + if attachments is not None and not isinstance(attachments, list): + raise ValueError("attachments must be a list of open files") if barcodes is not None: self.data["barcodes"] = [barcode.to_dict() for barcode in barcodes] diff --git a/rspace_client/tests/elnapi_test.py b/rspace_client/tests/elnapi_test.py index 0ddff33..1f6cc81 100755 --- a/rspace_client/tests/elnapi_test.py +++ b/rspace_client/tests/elnapi_test.py @@ -6,6 +6,9 @@ @author: richard """ import os, os.path +import tempfile + +import pytest import rspace_client.eln.eln as cli from rspace_client.eln.dcs import DocumentCreationStrategy @@ -15,6 +18,7 @@ from rspace_client.client_base import Pagination +@pytest.mark.integration class ELNClientAPIIntegrationTest(BaseApiTest): def setUp(self): self.assertClientCredentials() @@ -26,29 +30,34 @@ def test_get_status(self): def test_upload_downloadfile(self): file = get_datafile("fish_method.doc") + with tempfile.NamedTemporaryFile(suffix=".doc", delete=False) as tmp: + tmp_path = tmp.name try: with open(file, "rb") as to_upload: rs_file = self.api.upload_file(to_upload) - rs_get = self.api.download_file(rs_file["id"], "out.doc") + self.api.download_file(rs_file["id"], tmp_path) finally: - os.remove(os.path.join(os.getcwd(), "out.doc")) + if os.path.exists(tmp_path): + os.remove(tmp_path) def test_get_documents(self): + self.api.create_document(name=random_string(10)) resp = self.api.get_documents() self.assertTrue(resp["totalHits"] > 0) self.assertTrue(len(resp["documents"]) > 0) def test_stream_documents(self): + self.api.create_document(name=random_string(10)) + self.api.create_document(name=random_string(10)) doc_gen = self.api.stream_documents(pagination=Pagination(page_size=1)) d1 = next(doc_gen) d2 = next(doc_gen) self.assertNotEqual(d1["id"], d2["id"]) def test_get_documents_by_id(self): - resp = self.api.get_documents() - first_id = resp["documents"][0]["id"] - doc = self.api.get_document(first_id) - self.assertEqual(first_id, doc["id"]) + created = self.api.create_document(name=random_string(10)) + doc = self.api.get_document(created["id"]) + self.assertEqual(created["id"], doc["id"]) def test_create_document(self): nameStr = random_string(10) @@ -59,22 +68,25 @@ def test_create_document(self): def test_import_tree(self): tree_dir = get_datafile("tree") - res = self.api.import_tree(tree_dir) + folder = self.api.create_folder("tree-" + random_string(6)) + res = self.api.import_tree(tree_dir, parent_folder_id=folder["id"]) self.assertEqual("OK", res["status"]) ## f, 2sf, and 3files in each sf self.assertEqual(9, len(res["path2Id"].keys())) def test_import_tree_include_dot_files(self): tree_dir = get_datafile("tree") - res = self.api.import_tree(tree_dir, ignore_hidden_folders=False) + folder = self.api.create_folder("tree-" + random_string(6)) + res = self.api.import_tree(tree_dir, parent_folder_id=folder["id"], ignore_hidden_folders=False) self.assertEqual("OK", res["status"]) ## f, 2sf, and 3files in each sf + hidden self.assertTrue(len(res["path2Id"].keys()) >= 9) def test_import_tree_summary_doc_only(self): tree_dir = get_datafile("tree") + folder = self.api.create_folder("tree-" + random_string(6)) res = self.api.import_tree( - tree_dir, doc_creation=DocumentCreationStrategy.SUMMARY_DOC + tree_dir, parent_folder_id=folder["id"], doc_creation=DocumentCreationStrategy.SUMMARY_DOC ) self.assertEqual("OK", res["status"]) ## original folder + summary doc @@ -82,15 +94,16 @@ def test_import_tree_summary_doc_only(self): def test_import_tree_summary_doc_per_subfolder(self): tree_dir = get_datafile("tree") + folder = self.api.create_folder("tree-" + random_string(6)) res = self.api.import_tree( - tree_dir, doc_creation=DocumentCreationStrategy.DOC_PER_SUBFOLDER + tree_dir, parent_folder_id=folder["id"], doc_creation=DocumentCreationStrategy.DOC_PER_SUBFOLDER ) self.assertEqual("OK", res["status"]) ## original folder + 2 sf + 2 summary docs self.assertEqual(5, len(res["path2Id"].keys())) def test_import_tree_into_subfolder(self): - folder = self.api.create_folder("tree-root") + folder = self.api.create_folder("tree-root-" + random_string(6)) tree_dir = get_datafile("tree") res = self.api.import_tree(tree_dir, parent_folder_id=folder["id"]) self.assertEqual("OK", res["status"]) @@ -98,26 +111,31 @@ def test_import_tree_into_subfolder(self): self.assertEqual(9, len(res["path2Id"].keys())) def test_export_all_work_with_log_file(self): - file_path = "tmp-export.zip" - log_file = "tmp-log.txt" - try: + with tempfile.NamedTemporaryFile(suffix=".zip", delete=False) as f: + file_path = f.name + with tempfile.NamedTemporaryFile(suffix=".txt", delete=False) as f: + log_file = f.name + try: self.api.export_and_download("html", "user", file_path, progress_log=log_file, wait_between_requests=5) self.assertTrue(os.path.getsize(log_file) > 0) self.assertTrue(os.path.getsize(file_path) > 0) - except BaseException as e: - self.fail("Unexpected exception" + str(e)) + except Exception as e: + self.fail("Unexpected exception" + str(e)) finally: - os.remove(file_path) - os.remove(log_file) - + for p in (file_path, log_file): + if os.path.exists(p): + os.remove(p) + def test_export_all_work_with_no_file(self): - file_path = "tmp-export.zip" - try: - self.api.export_and_download("html", "user", file_path, wait_between_requests=5) - self.assertTrue(os.path.getsize(file_path) > 0) - except BaseException as e: - self.fail("Unexpected exception" + str(e)) - finally: - os.remove(file_path) + with tempfile.NamedTemporaryFile(suffix=".zip", delete=False) as f: + file_path = f.name + try: + self.api.export_and_download("html", "user", file_path, wait_between_requests=5) + self.assertTrue(os.path.getsize(file_path) > 0) + except Exception as e: + self.fail("Unexpected exception" + str(e)) + finally: + if os.path.exists(file_path): + os.remove(file_path) diff --git a/rspace_client/tests/inv_lom_test.py b/rspace_client/tests/inv_lom_test.py index 8c18cfc..fdc75d8 100644 --- a/rspace_client/tests/inv_lom_test.py +++ b/rspace_client/tests/inv_lom_test.py @@ -5,11 +5,14 @@ @author: richard """ +import pytest + import rspace_client.tests.base_test as base from rspace_client.inv import inv from rspace_client.eln import eln +@pytest.mark.integration class LomApiTest(base.BaseApiTest): def setUp(self): """ diff --git a/rspace_client/tests/invapi_test.py b/rspace_client/tests/invapi_test.py index 77392a6..14b221b 100644 --- a/rspace_client/tests/invapi_test.py +++ b/rspace_client/tests/invapi_test.py @@ -17,6 +17,7 @@ from rspace_client.inv import quantity_unit as qu +@pytest.mark.integration class InventoryApiTest(base.BaseApiTest): def setUp(self): """ @@ -57,7 +58,7 @@ def test_create_sample(self): def test_set_image_sample(self): sample = self.invapi.create_sample(base.random_string(5)) - file = base.get_datafile("AntibodySample150.png") + file = base.get_datafile("antibodySample150.png") with open(file, "rb") as f: updated_sample = self.invapi.set_image(sample, f) @@ -110,10 +111,11 @@ def test_rename_item(self): self.assertEqual(new_name, updated["name"]) def test_list_samples(self): + self.invapi.create_sample(base.random_string(5)) samples = self.invapi.list_samples(inv.Pagination(sort_order="desc")) self.assertEqual(0, samples["pageNumber"]) - self.assertEqual(10, len(samples["samples"])) + self.assertGreaterEqual(len(samples["samples"]), 1) def test_add_note_to_subsample(self): note = " a note about a subsample " + base.random_string() @@ -124,6 +126,9 @@ def test_add_note_to_subsample(self): self.assertEqual(note, updated["notes"][0]["content"]) def test_paginated_samples(self): + # Create 2 samples so page 1 (0-indexed) has results + self.invapi.create_sample(base.random_string(5)) + self.invapi.create_sample(base.random_string(5)) pag = inv.Pagination(page_number=1, page_size=1, sort_order="desc") samples = self.invapi.list_samples(pag) self.assertEqual(1, samples["pageNumber"]) @@ -136,9 +141,10 @@ def test_paginated_containers(self): c = self.invapi.set_as_top_level_container(c) containers = self.invapi.list_top_level_containers(pag) self.assertEqual(0, containers["pageNumber"]) - self.assertEqual(1, len(containers["containers"])) + self.assertGreaterEqual(len(containers["containers"]), 1) def test_paginated_subsamples(self): + self.invapi.create_sample(base.random_string(5), subsample_count=1) pag = inv.Pagination(page_number=0, page_size=1) ss = self.invapi.list_subsamples(pag) self.assertEqual(0, ss["pageNumber"]) @@ -162,6 +168,9 @@ def test_stream_containers(self): self.assertEqual(c2["id"], c2_l["id"]) def test_stream_samples(self): + # Ensure at least 2 samples exist + self.invapi.create_sample(base.random_string(5)) + self.invapi.create_sample(base.random_string(5)) onePerPage = inv.Pagination(page_size=1) gen = self.invapi.stream_samples(onePerPage) # get 2 items @@ -286,7 +295,7 @@ def test_duplicate(self): def test_get_benches(self): benches = self.invapi.get_workbenches() - self.assertEqual(2, len(benches)) + self.assertGreaterEqual(len(benches), 1) bench_ob = inv.Container.of(benches[0]) self.assertTrue(bench_ob.is_workbench()) @@ -679,12 +688,17 @@ def test_calculate_grid_start_validation(self): def test_barcode(self): barcode_bytes = self.invapi.barcode("SA14567") - self.assertEqual(99, len(barcode_bytes)) + self.assertTrue(len(barcode_bytes) > 0) + self.assertTrue(barcode_bytes.startswith(b"\x89PNG\r\n\x1a\n")) qr_bytes = self.invapi.barcode( - "SA12345", outfile="out10.png", barcode_type=inv.Barcode.QR + "SA12345", outfile="out10.png", barcode_type=inv.BarcodeFormat.QR ) - self.assertEqual(293, len(qr_bytes)) + self.assertTrue(len(qr_bytes) > 0) + self.assertTrue(qr_bytes.startswith(b"\x89PNG\r\n\x1a\n")) + self.assertTrue(os.path.exists("out10.png")) + self.assertTrue(os.path.getsize("out10.png") > 0) + os.remove("out10.png") def test_delete_samples(self): new_sample = self.invapi.create_sample("to_delete") diff --git a/rspace_client/tests/sample_post_test.py b/rspace_client/tests/sample_post_test.py new file mode 100644 index 0000000..75a185d --- /dev/null +++ b/rspace_client/tests/sample_post_test.py @@ -0,0 +1,59 @@ +#!/usr/bin/env python3 +# -*- coding: utf-8 -*- + +import unittest + +from rspace_client.inv import inv + + +class SamplePostTest(unittest.TestCase): + def test_sample_post_without_location_has_no_parent_fields(self): + post = inv.SamplePost("sample") + + self.assertNotIn("parentContainers", post.data) + self.assertNotIn("parentLocation", post.data) + self.assertNotIn("removeFromParentContainerRequest", post.data) + + def test_sample_post_with_list_container_location(self): + location = inv.ListContainerTargetLocation(123) + post = inv.SamplePost("sample", location=location) + + self.assertEqual([{"id": 123}], post.data["parentContainers"]) + self.assertNotIn("parentLocation", post.data) + + def test_sample_post_with_grid_location(self): + location = inv.GridContainerTargetLocation(123, 2, 3) + post = inv.SamplePost("sample", location=location) + + self.assertEqual([{"id": 123}], post.data["parentContainers"]) + self.assertEqual({"coordX": 2, "coordY": 3}, post.data["parentLocation"]) + + def test_sample_post_with_empty_barcode_request(self): + barcode = inv.Barcode(new_barcode_request=True) + post = inv.SamplePost("sample", barcodes=[barcode]) + + self.assertEqual( + [{"description": "", "newBarcodeRequest": True}], + post.data["barcodes"], + ) + + def test_sample_post_with_barcode_format_included_when_set(self): + barcode = inv.Barcode( + data="SA123", + format=inv.BarcodeFormat.QR, + description="test", + new_barcode_request=True, + ) + post = inv.SamplePost("sample", barcodes=[barcode]) + + self.assertEqual( + [ + { + "data": "SA123", + "format": "QR", + "description": "test", + "newBarcodeRequest": True, + } + ], + post.data["barcodes"], + ) From 8862045c72d7094c0283179e3edf960115176f31 Mon Sep 17 00:00:00 2001 From: nebay-abraha Date: Thu, 16 Apr 2026 15:16:01 +0100 Subject: [PATCH 02/14] remove c --- .github/scripts/get_rspace_api_key.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/scripts/get_rspace_api_key.py b/.github/scripts/get_rspace_api_key.py index 9351359..045cfbe 100644 --- a/.github/scripts/get_rspace_api_key.py +++ b/.github/scripts/get_rspace_api_key.py @@ -93,6 +93,6 @@ def main(): try: main() except Exception as exc: - print(f"Error generating API key: {exc}", file=sys.stderr)c + print(f"Error generating API key: {exc}", file=sys.stderr) sys.exit(1) \ No newline at end of file From 6c4ddc18d29bd057f5f7eb6d601ed8dc3ce78a39 Mon Sep 17 00:00:00 2001 From: nebay-abraha Date: Mon, 20 Apr 2026 13:37:42 +0100 Subject: [PATCH 03/14] remove unused imports --- .github/scripts/get_rspace_api_key.py | 2 +- rspace_client/inv/inv.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/scripts/get_rspace_api_key.py b/.github/scripts/get_rspace_api_key.py index 045cfbe..7c85b5e 100644 --- a/.github/scripts/get_rspace_api_key.py +++ b/.github/scripts/get_rspace_api_key.py @@ -1,7 +1,7 @@ import os import re import sys -from playwright.sync_api import sync_playwright, expect +from playwright.sync_api import sync_playwright from dotenv import load_dotenv load_dotenv() diff --git a/rspace_client/inv/inv.py b/rspace_client/inv/inv.py index 55bc652..1af5495 100644 --- a/rspace_client/inv/inv.py +++ b/rspace_client/inv/inv.py @@ -8,7 +8,7 @@ import requests import pprint import requests -from typing import Optional, Sequence, Union, List, TypedDict, BinaryIO, ClassVar +from typing import Optional, Sequence, Union, List, TypedDict, BinaryIO from rspace_client.client_base import ClientBase, Pagination from rspace_client.inv import quantity_unit as qu From a75dee295984d47bb2e39d5e3d18200acaaf4129 Mon Sep 17 00:00:00 2001 From: nebay-abraha Date: Mon, 20 Apr 2026 13:45:11 +0100 Subject: [PATCH 04/14] avoid including page text that may contain secret --- .github/scripts/get_rspace_api_key.py | 10 +++------- 1 file changed, 3 insertions(+), 7 deletions(-) diff --git a/.github/scripts/get_rspace_api_key.py b/.github/scripts/get_rspace_api_key.py index 7c85b5e..e899ec4 100644 --- a/.github/scripts/get_rspace_api_key.py +++ b/.github/scripts/get_rspace_api_key.py @@ -68,10 +68,8 @@ def main(): match = re.search(r"Key:\s*([A-Za-z0-9]{32})", info_text) if not match: - raise RuntimeError( - f"API key regex did not match — text length {len(info_text)}, starts with: {repr(info_text[:20])}" - ) - + raise RuntimeError(f"API key regex did not match for selector 'div.api-menu__key' — text length {len(info_text)}") + api_key = match.group(1) print("Successfully extracted API key", file=sys.stderr) @@ -88,11 +86,9 @@ def main(): browser.close() - if __name__ == "__main__": try: main() except Exception as exc: print(f"Error generating API key: {exc}", file=sys.stderr) - sys.exit(1) - \ No newline at end of file + sys.exit(1) \ No newline at end of file From b4bd5c5878c80e624ded010b6ce6d235ec69b1d0 Mon Sep 17 00:00:00 2001 From: nebay-abraha Date: Mon, 20 Apr 2026 13:52:38 +0100 Subject: [PATCH 05/14] update analyze job permissions. --- .github/workflows/codeql-and-tests.yml | 2 ++ 1 file changed, 2 insertions(+) diff --git a/.github/workflows/codeql-and-tests.yml b/.github/workflows/codeql-and-tests.yml index 9b8d063..dc7394e 100644 --- a/.github/workflows/codeql-and-tests.yml +++ b/.github/workflows/codeql-and-tests.yml @@ -16,6 +16,8 @@ jobs: runs-on: ubuntu-24.04 timeout-minutes: 20 permissions: + actions: read + contents: read security-events: write strategy: fail-fast: false From b554e3278f76840478a2dd3a532037c507e29120 Mon Sep 17 00:00:00 2001 From: nebay-abraha Date: Mon, 20 Apr 2026 14:37:57 +0100 Subject: [PATCH 06/14] revert barcode dataclass --- rspace_client/inv/inv.py | 19 ++++++++----------- rspace_client/tests/sample_post_test.py | 11 +---------- 2 files changed, 9 insertions(+), 21 deletions(-) diff --git a/rspace_client/inv/inv.py b/rspace_client/inv/inv.py index 1af5495..a857b54 100644 --- a/rspace_client/inv/inv.py +++ b/rspace_client/inv/inv.py @@ -33,22 +33,19 @@ class Tag(TypedDict): @dataclass class Barcode: - data: Optional[str] = None - format: Optional[BarcodeFormat] = None + data: str + format: BarcodeFormat description: str = "" - new_barcode_request: bool = True - id: Optional[str] = None + newBarcodeRequest: bool = True + id: Optional[str] = "" def to_dict(self): - result = { + return{ + "data": self.data, + "format": self.format.value, "description": self.description, - "newBarcodeRequest": self.new_barcode_request, + "newBarcodeRequest": self.newBarcodeRequest } - if self.data is not None: - result["data"] = self.data - if self.format is not None: - result["format"] = self.format.value - return result class FillingStrategy(Enum): diff --git a/rspace_client/tests/sample_post_test.py b/rspace_client/tests/sample_post_test.py index 75a185d..be5f95b 100644 --- a/rspace_client/tests/sample_post_test.py +++ b/rspace_client/tests/sample_post_test.py @@ -28,21 +28,12 @@ def test_sample_post_with_grid_location(self): self.assertEqual([{"id": 123}], post.data["parentContainers"]) self.assertEqual({"coordX": 2, "coordY": 3}, post.data["parentLocation"]) - def test_sample_post_with_empty_barcode_request(self): - barcode = inv.Barcode(new_barcode_request=True) - post = inv.SamplePost("sample", barcodes=[barcode]) - - self.assertEqual( - [{"description": "", "newBarcodeRequest": True}], - post.data["barcodes"], - ) - def test_sample_post_with_barcode_format_included_when_set(self): barcode = inv.Barcode( data="SA123", format=inv.BarcodeFormat.QR, description="test", - new_barcode_request=True, + newBarcodeRequest=True, ) post = inv.SamplePost("sample", barcodes=[barcode]) From 90efde71017451e70c9c3c96706572cc563b8f89 Mon Sep 17 00:00:00 2001 From: nebay-abraha Date: Sun, 5 Jul 2026 09:46:12 +0100 Subject: [PATCH 07/14] remove rspace-docker, playwright and rely on rspace-web project to init rspace instance --- .env.example | 5 +- .github/scripts/get_rspace_api_key.py | 94 ----------------- .github/workflows/codeql-and-tests.yml | 134 ++++++++++++++++++------- DEVELOPING.md | 5 + pyproject.toml | 1 - 5 files changed, 104 insertions(+), 135 deletions(-) delete mode 100644 .github/scripts/get_rspace_api_key.py diff --git a/.env.example b/.env.example index d0a1977..8469ca8 100644 --- a/.env.example +++ b/.env.example @@ -1,5 +1,2 @@ RSPACE_URL= -RSPACE_API_KEY= - -RSPACE_USERNAME= -RSPACE_PASSWORD= \ No newline at end of file +RSPACE_API_KEY= \ No newline at end of file diff --git a/.github/scripts/get_rspace_api_key.py b/.github/scripts/get_rspace_api_key.py deleted file mode 100644 index e899ec4..0000000 --- a/.github/scripts/get_rspace_api_key.py +++ /dev/null @@ -1,94 +0,0 @@ -import os -import re -import sys -from playwright.sync_api import sync_playwright -from dotenv import load_dotenv - -load_dotenv() - -RSPACE_URL = os.getenv("RSPACE_URL") -RSPACE_USERNAME = os.getenv("RSPACE_USERNAME") -RSPACE_PASSWORD = os.getenv("RSPACE_PASSWORD") - - -def main(): - if not RSPACE_URL or not RSPACE_USERNAME or not RSPACE_PASSWORD: - raise RuntimeError( - "Missing required environment variables: RSPACE_URL, RSPACE_USERNAME, RSPACE_PASSWORD" - ) - - with sync_playwright() as p: - browser = p.chromium.launch(headless=True) - context = browser.new_context() - page = context.new_page() - - print("Step 1: Navigating to RSpace...", file=sys.stderr) - page.goto(RSPACE_URL, wait_until="networkidle") - - print("Step 2: Logging in...", file=sys.stderr) - page.get_by_role("textbox", name="User").fill(RSPACE_USERNAME) - page.get_by_role("textbox", name="Password").fill(RSPACE_PASSWORD) - page.get_by_role("button", name="Log in").click() - - # Wait for login to complete and redirect - page.wait_for_load_state("networkidle") - print("Step 3: Login successful, navigating to My RSpace...", file=sys.stderr) - - # Step 2: Navigate to Profile → Manage API Key - page.get_by_role("link", name="My RSpace").click() - page.wait_for_load_state("networkidle") - - print("Step 4: Looking for Generate/Regenerate key button...", file=sys.stderr) - # Wait for the button to appear (either "Generate key" or "Regenerate key") - page.wait_for_selector("a#apiKeyRegenerateBtn", timeout=10000) - page.click("a#apiKeyRegenerateBtn") - - print("Step 5: Confirming password...", file=sys.stderr) - dialog = page.get_by_role("dialog", name="Confirm password") - dialog.wait_for() - dialog.get_by_role("textbox", name="Please confirm your password").fill(RSPACE_PASSWORD) - dialog.get_by_role("button", name="OK").click() - dialog.wait_for(state="detached") - - - print("Step 6: Waiting for key to be displayed...", file=sys.stderr) - page.wait_for_load_state("networkidle") - - # Wait for the key to appear in the page - page.wait_for_selector("#apiKeyInfo") - - # Extract the key from the displayed text - # Format: "Key: {32-char-string}" - key_locator = page.locator("div.api-menu__key") - if key_locator.count() == 0: - raise RuntimeError("Selector 'div.api-menu__key' not found on page — page structure may have changed") - - info_text = key_locator.inner_text() - print(f"Key element text length: {len(info_text)}", file=sys.stderr) - - match = re.search(r"Key:\s*([A-Za-z0-9]{32})", info_text) - if not match: - raise RuntimeError(f"API key regex did not match for selector 'div.api-menu__key' — text length {len(info_text)}") - - api_key = match.group(1) - print("Successfully extracted API key", file=sys.stderr) - - # Write to GITHUB_OUTPUT so the key never touches stdout/stderr. - # Writing to GITHUB_OUTPUT is the standard GitHub Actions mechanism for passing - # step outputs; the file is ephemeral and runner-scoped. - github_output = os.getenv("GITHUB_OUTPUT") - if github_output: - with open(github_output, "a") as f: - f.write(f"rspace_api_key={api_key}\n") - else: - raise RuntimeError("GITHUB_OUTPUT is not set; refusing to emit API key to stdout.") - - browser.close() - - -if __name__ == "__main__": - try: - main() - except Exception as exc: - print(f"Error generating API key: {exc}", file=sys.stderr) - sys.exit(1) \ No newline at end of file diff --git a/.github/workflows/codeql-and-tests.yml b/.github/workflows/codeql-and-tests.yml index dc7394e..7d3b373 100644 --- a/.github/workflows/codeql-and-tests.yml +++ b/.github/workflows/codeql-and-tests.yml @@ -79,11 +79,42 @@ jobs: needs: test permissions: contents: read + env: + RS_FILE_BASE: /tmp/e2e-filestore + services: + db: + image: mariadb:lts-jammy + env: + MARIADB_ROOT_PASSWORD: rspacedbpwd + MARIADB_DATABASE: rspace + MARIADB_USER: rspacedbuser + MARIADB_PASSWORD: rspacedbpwd + ports: + - 3306:3306 + options: >- + --tmpfs /var/lib/mysql:rw,noexec,nosuid,size=2g + --health-cmd="healthcheck.sh --connect --innodb_initialized" + --health-interval=5s + --health-timeout=5s + --health-retries=40 + --health-start-period=30s + # RSpace's chemistry-gated integrations only render when this is reachable. + chemistry: + image: rspaceops/oss-chemistry:latest + ports: + - 8090:8090 steps: - - name: Checkout repository + - name: Checkout rspace-client-python uses: actions/checkout@v4 + - name: Checkout rspace-web + uses: actions/checkout@v4 + with: + repository: rspace-os/rspace-web + path: rspace-web + persist-credentials: false + - name: Set up Python 3.12 uses: actions/setup-python@v5 with: @@ -99,13 +130,20 @@ jobs: - name: Install dependencies run: poetry install --no-interaction - - name: Clone rspace-docker - run: git clone https://github.com/rspace-os/rspace-docker.git /tmp/rspace-docker + - name: Set up JDK + uses: actions/setup-java@v5 + with: + distribution: temurin + java-version: 17 + cache: maven + cache-dependency-path: rspace-web/pom.xml - name: Download latest RSpace WAR + working-directory: rspace-web run: | set -euo pipefail + mkdir -p target release_json=$(curl -fsSL https://api.github.com/repos/rspace-os/rspace-web/releases/latest) latest_tag=$(echo "$release_json" | jq -r '.tag_name') war_url=$(echo "$release_json" | jq -r '.assets[]? | select(.name | test("^researchspace-.*\\.war$")) | .browser_download_url' | head -n1) @@ -120,7 +158,7 @@ jobs: fi echo "Latest RSpace tag: $latest_tag" - curl -fsSL "$war_url" -o /tmp/rspace-docker/rspace.war + curl -fsSL "$war_url" -o target/researchspace.war - name: Free up disk space run: | @@ -129,44 +167,68 @@ jobs: docker image prune -af || true df -h - - name: Start RSpace - working-directory: /tmp/rspace-docker - run: docker compose up -d - - - name: Wait for RSpace to be ready - timeout-minutes: 10 + - name: Configure MariaDB + run: | + mysql -h127.0.0.1 -uroot -prspacedbpwd <<'SQL' + SET GLOBAL character_set_server = 'utf8mb4'; + SET GLOBAL collation_server = 'utf8mb4_unicode_ci'; + SET GLOBAL sql_mode = 'STRICT_TRANS_TABLES,NO_ZERO_IN_DATE,NO_ZERO_DATE,ERROR_FOR_DIVISION_BY_ZERO,NO_ENGINE_SUBSTITUTION'; + GRANT CREATE, DROP ON *.* TO 'rspacedbuser'@'%'; + FLUSH PRIVILEGES; + SQL + + - name: Setup filestore + run: | + rm -rf "$RS_FILE_BASE" + mkdir -p "$RS_FILE_BASE" + + # Deploys the downloaded WAR directly (-Dmaven.war.skip=true skips + # rebuilding it) against a "dev" + "drop-recreate-db" environment, which + # drops/recreates the schema and loads RSpace's dev-test Liquibase seed + # data - including a sysadmin1 user with a fixed, known API key. No + # separate key-generation step (UI or REST) is needed as a result. + - name: Start RSpace (prebuilt WAR) and wait for readiness + working-directory: rspace-web run: | set -euo pipefail - - echo "Waiting for RSpace on http://localhost:8080 ..." - for i in {1..40}; do - if timeout 2 bash -c "cat < /dev/null > /dev/tcp/localhost/8080" 2>/dev/null; then - echo "RSpace is up" - exit 0 + nohup ./mvnw jetty:run-war \ + -DskipTests=true -Dmaven.war.skip=true \ + -Dtimestamp=${{ github.run_id }} \ + -Djava-version=17 -Djava-vendor=temurin \ + -Denvironment=drop-recreate-db \ + -Dspring.profiles.active=run \ + -DreactDevMode=false \ + -Dchemistry.provider=indigo \ + -Dchemistry.service.url=http://localhost:8090 \ + -Djdbc.url=jdbc:mysql://localhost:3306/rspace \ + -Djdbc.db.maven=rspace \ + -DRS_FILE_BASE="$RS_FILE_BASE" \ + -DRS.devlogLevel=INFO \ + > /tmp/jetty.log 2>&1 & + for i in $(seq 1 180); do + if [ "$(curl -s -o /dev/null -w '%{http_code}' http://localhost:8080/login || true)" = "200" ]; then + echo "Ready after ~$((i*5))s"; exit 0 fi - echo " attempt $i/40..." - sleep 15 + sleep 5 done - echo "RSpace failed to start" - exit 1 - - - name: Install Playwright Browsers - run: poetry run python -m playwright install chromium --with-deps - - - name: Generate RSpace API Key - id: generate_key - run: poetry run python .github/scripts/get_rspace_api_key.py - env: - RSPACE_URL: http://localhost:8080 - RSPACE_USERNAME: ${{ secrets.RSPACE_USERNAME }} - RSPACE_PASSWORD: ${{ secrets.RSPACE_PASSWORD }} - - - name: Mask API key - run: echo "::add-mask::${{ steps.generate_key.outputs.rspace_api_key }}" + echo "::error::RSpace did not become ready in time"; tail -100 /tmp/jetty.log; exit 1 - name: Run integration tests timeout-minutes: 20 env: RSPACE_URL: http://localhost:8080 - RSPACE_API_KEY: ${{ steps.generate_key.outputs.rspace_api_key }} - run: poetry run pytest -m integration -v \ No newline at end of file + # Pre-seeded sysadmin1 API key from RSpace's dev-test Liquibase seed data. + RSPACE_API_KEY: abcdefghijklmnop12 + run: poetry run pytest -m integration -v + + - name: Dump server log on failure + if: failure() + run: tail -200 /tmp/jetty.log || true + + - name: Upload Jetty log on failure + if: failure() + uses: actions/upload-artifact@v4 + with: + name: jetty-log + path: /tmp/jetty.log + retention-days: 3 \ No newline at end of file diff --git a/DEVELOPING.md b/DEVELOPING.md index 062200f..a6fa83d 100644 --- a/DEVELOPING.md +++ b/DEVELOPING.md @@ -45,6 +45,11 @@ poetry run pytest -m integration ``` Integration tests should be run with a new RSpace account that does not belong to any groups. + +In CI, the `integration-test` job boots RSpace the same way `rspace-web`'s `e2e.yml` does: it downloads +the latest release WAR, deploys it with `mvnw jetty:run-war -Denvironment=drop-recreate-db` against a +MariaDB service container, which loads RSpace's dev-test seed data. That seed data includes a `sysadmin1` +user with a fixed, known API key, so no key-generation step is needed. ### Writing Tests diff --git a/pyproject.toml b/pyproject.toml index f7325aa..ed05d22 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -31,7 +31,6 @@ setuptools = "<82" python-dotenv = "^1.1.1" black = "^21.6b0" pytest = "^8.0.0" -playwright = "^1.58.0" [build-system] requires = ["poetry-core>=1.0.0"] From bfedef42c035756a2f344a321120e0d6b2939d45 Mon Sep 17 00:00:00 2001 From: nebay-abraha Date: Sun, 5 Jul 2026 10:28:53 +0100 Subject: [PATCH 08/14] Revert "remove rspace-docker, playwright and rely on rspace-web project to init rspace instance" This reverts commit e3ab7228055078f883873859f846aebacbbb48d0. Replicating rspace-web's e2e.yml boot mechanism required a full Maven/Node rebuild of rspace-web on every CI run (jetty:run-war forks a full "package" phase regardless of -Dmaven.war.skip), which is far more expensive than the docker-compose flow it replaced. Reverting to the known-working setup. Co-Authored-By: Claude Sonnet 5 --- .env.example | 5 +- .github/scripts/get_rspace_api_key.py | 94 +++++++++++++++++ .github/workflows/codeql-and-tests.yml | 134 +++++++------------------ DEVELOPING.md | 5 - pyproject.toml | 1 + 5 files changed, 135 insertions(+), 104 deletions(-) create mode 100644 .github/scripts/get_rspace_api_key.py diff --git a/.env.example b/.env.example index 8469ca8..d0a1977 100644 --- a/.env.example +++ b/.env.example @@ -1,2 +1,5 @@ RSPACE_URL= -RSPACE_API_KEY= \ No newline at end of file +RSPACE_API_KEY= + +RSPACE_USERNAME= +RSPACE_PASSWORD= \ No newline at end of file diff --git a/.github/scripts/get_rspace_api_key.py b/.github/scripts/get_rspace_api_key.py new file mode 100644 index 0000000..e899ec4 --- /dev/null +++ b/.github/scripts/get_rspace_api_key.py @@ -0,0 +1,94 @@ +import os +import re +import sys +from playwright.sync_api import sync_playwright +from dotenv import load_dotenv + +load_dotenv() + +RSPACE_URL = os.getenv("RSPACE_URL") +RSPACE_USERNAME = os.getenv("RSPACE_USERNAME") +RSPACE_PASSWORD = os.getenv("RSPACE_PASSWORD") + + +def main(): + if not RSPACE_URL or not RSPACE_USERNAME or not RSPACE_PASSWORD: + raise RuntimeError( + "Missing required environment variables: RSPACE_URL, RSPACE_USERNAME, RSPACE_PASSWORD" + ) + + with sync_playwright() as p: + browser = p.chromium.launch(headless=True) + context = browser.new_context() + page = context.new_page() + + print("Step 1: Navigating to RSpace...", file=sys.stderr) + page.goto(RSPACE_URL, wait_until="networkidle") + + print("Step 2: Logging in...", file=sys.stderr) + page.get_by_role("textbox", name="User").fill(RSPACE_USERNAME) + page.get_by_role("textbox", name="Password").fill(RSPACE_PASSWORD) + page.get_by_role("button", name="Log in").click() + + # Wait for login to complete and redirect + page.wait_for_load_state("networkidle") + print("Step 3: Login successful, navigating to My RSpace...", file=sys.stderr) + + # Step 2: Navigate to Profile → Manage API Key + page.get_by_role("link", name="My RSpace").click() + page.wait_for_load_state("networkidle") + + print("Step 4: Looking for Generate/Regenerate key button...", file=sys.stderr) + # Wait for the button to appear (either "Generate key" or "Regenerate key") + page.wait_for_selector("a#apiKeyRegenerateBtn", timeout=10000) + page.click("a#apiKeyRegenerateBtn") + + print("Step 5: Confirming password...", file=sys.stderr) + dialog = page.get_by_role("dialog", name="Confirm password") + dialog.wait_for() + dialog.get_by_role("textbox", name="Please confirm your password").fill(RSPACE_PASSWORD) + dialog.get_by_role("button", name="OK").click() + dialog.wait_for(state="detached") + + + print("Step 6: Waiting for key to be displayed...", file=sys.stderr) + page.wait_for_load_state("networkidle") + + # Wait for the key to appear in the page + page.wait_for_selector("#apiKeyInfo") + + # Extract the key from the displayed text + # Format: "Key: {32-char-string}" + key_locator = page.locator("div.api-menu__key") + if key_locator.count() == 0: + raise RuntimeError("Selector 'div.api-menu__key' not found on page — page structure may have changed") + + info_text = key_locator.inner_text() + print(f"Key element text length: {len(info_text)}", file=sys.stderr) + + match = re.search(r"Key:\s*([A-Za-z0-9]{32})", info_text) + if not match: + raise RuntimeError(f"API key regex did not match for selector 'div.api-menu__key' — text length {len(info_text)}") + + api_key = match.group(1) + print("Successfully extracted API key", file=sys.stderr) + + # Write to GITHUB_OUTPUT so the key never touches stdout/stderr. + # Writing to GITHUB_OUTPUT is the standard GitHub Actions mechanism for passing + # step outputs; the file is ephemeral and runner-scoped. + github_output = os.getenv("GITHUB_OUTPUT") + if github_output: + with open(github_output, "a") as f: + f.write(f"rspace_api_key={api_key}\n") + else: + raise RuntimeError("GITHUB_OUTPUT is not set; refusing to emit API key to stdout.") + + browser.close() + + +if __name__ == "__main__": + try: + main() + except Exception as exc: + print(f"Error generating API key: {exc}", file=sys.stderr) + sys.exit(1) \ No newline at end of file diff --git a/.github/workflows/codeql-and-tests.yml b/.github/workflows/codeql-and-tests.yml index 7d3b373..dc7394e 100644 --- a/.github/workflows/codeql-and-tests.yml +++ b/.github/workflows/codeql-and-tests.yml @@ -79,41 +79,10 @@ jobs: needs: test permissions: contents: read - env: - RS_FILE_BASE: /tmp/e2e-filestore - services: - db: - image: mariadb:lts-jammy - env: - MARIADB_ROOT_PASSWORD: rspacedbpwd - MARIADB_DATABASE: rspace - MARIADB_USER: rspacedbuser - MARIADB_PASSWORD: rspacedbpwd - ports: - - 3306:3306 - options: >- - --tmpfs /var/lib/mysql:rw,noexec,nosuid,size=2g - --health-cmd="healthcheck.sh --connect --innodb_initialized" - --health-interval=5s - --health-timeout=5s - --health-retries=40 - --health-start-period=30s - # RSpace's chemistry-gated integrations only render when this is reachable. - chemistry: - image: rspaceops/oss-chemistry:latest - ports: - - 8090:8090 steps: - - name: Checkout rspace-client-python - uses: actions/checkout@v4 - - - name: Checkout rspace-web + - name: Checkout repository uses: actions/checkout@v4 - with: - repository: rspace-os/rspace-web - path: rspace-web - persist-credentials: false - name: Set up Python 3.12 uses: actions/setup-python@v5 @@ -130,20 +99,13 @@ jobs: - name: Install dependencies run: poetry install --no-interaction - - name: Set up JDK - uses: actions/setup-java@v5 - with: - distribution: temurin - java-version: 17 - cache: maven - cache-dependency-path: rspace-web/pom.xml + - name: Clone rspace-docker + run: git clone https://github.com/rspace-os/rspace-docker.git /tmp/rspace-docker - name: Download latest RSpace WAR - working-directory: rspace-web run: | set -euo pipefail - mkdir -p target release_json=$(curl -fsSL https://api.github.com/repos/rspace-os/rspace-web/releases/latest) latest_tag=$(echo "$release_json" | jq -r '.tag_name') war_url=$(echo "$release_json" | jq -r '.assets[]? | select(.name | test("^researchspace-.*\\.war$")) | .browser_download_url' | head -n1) @@ -158,7 +120,7 @@ jobs: fi echo "Latest RSpace tag: $latest_tag" - curl -fsSL "$war_url" -o target/researchspace.war + curl -fsSL "$war_url" -o /tmp/rspace-docker/rspace.war - name: Free up disk space run: | @@ -167,68 +129,44 @@ jobs: docker image prune -af || true df -h - - name: Configure MariaDB - run: | - mysql -h127.0.0.1 -uroot -prspacedbpwd <<'SQL' - SET GLOBAL character_set_server = 'utf8mb4'; - SET GLOBAL collation_server = 'utf8mb4_unicode_ci'; - SET GLOBAL sql_mode = 'STRICT_TRANS_TABLES,NO_ZERO_IN_DATE,NO_ZERO_DATE,ERROR_FOR_DIVISION_BY_ZERO,NO_ENGINE_SUBSTITUTION'; - GRANT CREATE, DROP ON *.* TO 'rspacedbuser'@'%'; - FLUSH PRIVILEGES; - SQL - - - name: Setup filestore - run: | - rm -rf "$RS_FILE_BASE" - mkdir -p "$RS_FILE_BASE" - - # Deploys the downloaded WAR directly (-Dmaven.war.skip=true skips - # rebuilding it) against a "dev" + "drop-recreate-db" environment, which - # drops/recreates the schema and loads RSpace's dev-test Liquibase seed - # data - including a sysadmin1 user with a fixed, known API key. No - # separate key-generation step (UI or REST) is needed as a result. - - name: Start RSpace (prebuilt WAR) and wait for readiness - working-directory: rspace-web + - name: Start RSpace + working-directory: /tmp/rspace-docker + run: docker compose up -d + + - name: Wait for RSpace to be ready + timeout-minutes: 10 run: | set -euo pipefail - nohup ./mvnw jetty:run-war \ - -DskipTests=true -Dmaven.war.skip=true \ - -Dtimestamp=${{ github.run_id }} \ - -Djava-version=17 -Djava-vendor=temurin \ - -Denvironment=drop-recreate-db \ - -Dspring.profiles.active=run \ - -DreactDevMode=false \ - -Dchemistry.provider=indigo \ - -Dchemistry.service.url=http://localhost:8090 \ - -Djdbc.url=jdbc:mysql://localhost:3306/rspace \ - -Djdbc.db.maven=rspace \ - -DRS_FILE_BASE="$RS_FILE_BASE" \ - -DRS.devlogLevel=INFO \ - > /tmp/jetty.log 2>&1 & - for i in $(seq 1 180); do - if [ "$(curl -s -o /dev/null -w '%{http_code}' http://localhost:8080/login || true)" = "200" ]; then - echo "Ready after ~$((i*5))s"; exit 0 + + echo "Waiting for RSpace on http://localhost:8080 ..." + for i in {1..40}; do + if timeout 2 bash -c "cat < /dev/null > /dev/tcp/localhost/8080" 2>/dev/null; then + echo "RSpace is up" + exit 0 fi - sleep 5 + echo " attempt $i/40..." + sleep 15 done - echo "::error::RSpace did not become ready in time"; tail -100 /tmp/jetty.log; exit 1 + echo "RSpace failed to start" + exit 1 - - name: Run integration tests - timeout-minutes: 20 + - name: Install Playwright Browsers + run: poetry run python -m playwright install chromium --with-deps + + - name: Generate RSpace API Key + id: generate_key + run: poetry run python .github/scripts/get_rspace_api_key.py env: RSPACE_URL: http://localhost:8080 - # Pre-seeded sysadmin1 API key from RSpace's dev-test Liquibase seed data. - RSPACE_API_KEY: abcdefghijklmnop12 - run: poetry run pytest -m integration -v + RSPACE_USERNAME: ${{ secrets.RSPACE_USERNAME }} + RSPACE_PASSWORD: ${{ secrets.RSPACE_PASSWORD }} - - name: Dump server log on failure - if: failure() - run: tail -200 /tmp/jetty.log || true + - name: Mask API key + run: echo "::add-mask::${{ steps.generate_key.outputs.rspace_api_key }}" - - name: Upload Jetty log on failure - if: failure() - uses: actions/upload-artifact@v4 - with: - name: jetty-log - path: /tmp/jetty.log - retention-days: 3 \ No newline at end of file + - name: Run integration tests + timeout-minutes: 20 + env: + RSPACE_URL: http://localhost:8080 + RSPACE_API_KEY: ${{ steps.generate_key.outputs.rspace_api_key }} + run: poetry run pytest -m integration -v \ No newline at end of file diff --git a/DEVELOPING.md b/DEVELOPING.md index a6fa83d..062200f 100644 --- a/DEVELOPING.md +++ b/DEVELOPING.md @@ -45,11 +45,6 @@ poetry run pytest -m integration ``` Integration tests should be run with a new RSpace account that does not belong to any groups. - -In CI, the `integration-test` job boots RSpace the same way `rspace-web`'s `e2e.yml` does: it downloads -the latest release WAR, deploys it with `mvnw jetty:run-war -Denvironment=drop-recreate-db` against a -MariaDB service container, which loads RSpace's dev-test seed data. That seed data includes a `sysadmin1` -user with a fixed, known API key, so no key-generation step is needed. ### Writing Tests diff --git a/pyproject.toml b/pyproject.toml index ed05d22..f7325aa 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -31,6 +31,7 @@ setuptools = "<82" python-dotenv = "^1.1.1" black = "^21.6b0" pytest = "^8.0.0" +playwright = "^1.58.0" [build-system] requires = ["poetry-core>=1.0.0"] From 3d13daebdb0a1cbbe8b29dd6ad7554eac66bfccb Mon Sep 17 00:00:00 2001 From: nebay-abraha Date: Tue, 7 Jul 2026 20:08:12 +0100 Subject: [PATCH 09/14] replace playwright + rspace-docker with rspace build in ci this avoids constant failure due to ui changes which playwright ui depends on --- .env.example | 2 - .github/scripts/get_rspace_api_key.py | 94 ---------------- .github/scripts/warmup_sysadmin.py | 48 ++++++++ .github/workflows/codeql-and-tests.yml | 150 +++++++++++++++++-------- pyproject.toml | 1 - 5 files changed, 151 insertions(+), 144 deletions(-) delete mode 100644 .github/scripts/get_rspace_api_key.py create mode 100644 .github/scripts/warmup_sysadmin.py diff --git a/.env.example b/.env.example index d0a1977..67ab854 100644 --- a/.env.example +++ b/.env.example @@ -1,5 +1,3 @@ RSPACE_URL= RSPACE_API_KEY= -RSPACE_USERNAME= -RSPACE_PASSWORD= \ No newline at end of file diff --git a/.github/scripts/get_rspace_api_key.py b/.github/scripts/get_rspace_api_key.py deleted file mode 100644 index e899ec4..0000000 --- a/.github/scripts/get_rspace_api_key.py +++ /dev/null @@ -1,94 +0,0 @@ -import os -import re -import sys -from playwright.sync_api import sync_playwright -from dotenv import load_dotenv - -load_dotenv() - -RSPACE_URL = os.getenv("RSPACE_URL") -RSPACE_USERNAME = os.getenv("RSPACE_USERNAME") -RSPACE_PASSWORD = os.getenv("RSPACE_PASSWORD") - - -def main(): - if not RSPACE_URL or not RSPACE_USERNAME or not RSPACE_PASSWORD: - raise RuntimeError( - "Missing required environment variables: RSPACE_URL, RSPACE_USERNAME, RSPACE_PASSWORD" - ) - - with sync_playwright() as p: - browser = p.chromium.launch(headless=True) - context = browser.new_context() - page = context.new_page() - - print("Step 1: Navigating to RSpace...", file=sys.stderr) - page.goto(RSPACE_URL, wait_until="networkidle") - - print("Step 2: Logging in...", file=sys.stderr) - page.get_by_role("textbox", name="User").fill(RSPACE_USERNAME) - page.get_by_role("textbox", name="Password").fill(RSPACE_PASSWORD) - page.get_by_role("button", name="Log in").click() - - # Wait for login to complete and redirect - page.wait_for_load_state("networkidle") - print("Step 3: Login successful, navigating to My RSpace...", file=sys.stderr) - - # Step 2: Navigate to Profile → Manage API Key - page.get_by_role("link", name="My RSpace").click() - page.wait_for_load_state("networkidle") - - print("Step 4: Looking for Generate/Regenerate key button...", file=sys.stderr) - # Wait for the button to appear (either "Generate key" or "Regenerate key") - page.wait_for_selector("a#apiKeyRegenerateBtn", timeout=10000) - page.click("a#apiKeyRegenerateBtn") - - print("Step 5: Confirming password...", file=sys.stderr) - dialog = page.get_by_role("dialog", name="Confirm password") - dialog.wait_for() - dialog.get_by_role("textbox", name="Please confirm your password").fill(RSPACE_PASSWORD) - dialog.get_by_role("button", name="OK").click() - dialog.wait_for(state="detached") - - - print("Step 6: Waiting for key to be displayed...", file=sys.stderr) - page.wait_for_load_state("networkidle") - - # Wait for the key to appear in the page - page.wait_for_selector("#apiKeyInfo") - - # Extract the key from the displayed text - # Format: "Key: {32-char-string}" - key_locator = page.locator("div.api-menu__key") - if key_locator.count() == 0: - raise RuntimeError("Selector 'div.api-menu__key' not found on page — page structure may have changed") - - info_text = key_locator.inner_text() - print(f"Key element text length: {len(info_text)}", file=sys.stderr) - - match = re.search(r"Key:\s*([A-Za-z0-9]{32})", info_text) - if not match: - raise RuntimeError(f"API key regex did not match for selector 'div.api-menu__key' — text length {len(info_text)}") - - api_key = match.group(1) - print("Successfully extracted API key", file=sys.stderr) - - # Write to GITHUB_OUTPUT so the key never touches stdout/stderr. - # Writing to GITHUB_OUTPUT is the standard GitHub Actions mechanism for passing - # step outputs; the file is ephemeral and runner-scoped. - github_output = os.getenv("GITHUB_OUTPUT") - if github_output: - with open(github_output, "a") as f: - f.write(f"rspace_api_key={api_key}\n") - else: - raise RuntimeError("GITHUB_OUTPUT is not set; refusing to emit API key to stdout.") - - browser.close() - - -if __name__ == "__main__": - try: - main() - except Exception as exc: - print(f"Error generating API key: {exc}", file=sys.stderr) - sys.exit(1) \ No newline at end of file diff --git a/.github/scripts/warmup_sysadmin.py b/.github/scripts/warmup_sysadmin.py new file mode 100644 index 0000000..610603a --- /dev/null +++ b/.github/scripts/warmup_sysadmin.py @@ -0,0 +1,48 @@ +import os +import sys +import time + +import requests + +RSPACE_URL = os.environ["RSPACE_URL"] + +# Public, non-secret bootstrap credentials seeded by rspace-web's own "dev-test" +# Liquibase context (initial-seed-run.sql / initial-seed-devtest.sql) for the +# built-in sysadmin1 account. +SYSADMIN_USERNAME = "sysadmin1" +SYSADMIN_PASSWORD = "sysWisc23!" + +# A user account that is only ever authenticated via its API key (never a real +# login) has its home folder created lazily on first write - a code path that +# throws on that first attempt (though it leaves the account usable afterward). +# A real login runs the same initialization synchronously and correctly, so a +# single login here avoids ever hitting that first-write bug during the test run. +def warm_up(timeout_seconds=300, interval_seconds=5): + deadline = time.monotonic() + timeout_seconds + session = requests.Session() + last_error = None + while time.monotonic() < deadline: + try: + session.get(f"{RSPACE_URL}/login", timeout=10) + resp = session.post( + f"{RSPACE_URL}/login", + data={"username": SYSADMIN_USERNAME, "password": SYSADMIN_PASSWORD}, + timeout=10, + allow_redirects=True, + ) + if resp.status_code == 200 and "loginForm" not in resp.text: + return + last_error = f"HTTP {resp.status_code}, login form still present" + except requests.exceptions.RequestException as exc: + last_error = str(exc) + print(f"Login not successful yet ({last_error}); retrying...", file=sys.stderr) + time.sleep(interval_seconds) + raise RuntimeError(f"Could not log in as {SYSADMIN_USERNAME}: {last_error}") + + +if __name__ == "__main__": + try: + warm_up() + except Exception as exc: + print(f"Error warming up sysadmin1 account: {exc}", file=sys.stderr) + sys.exit(1) diff --git a/.github/workflows/codeql-and-tests.yml b/.github/workflows/codeql-and-tests.yml index dc7394e..5864831 100644 --- a/.github/workflows/codeql-and-tests.yml +++ b/.github/workflows/codeql-and-tests.yml @@ -79,6 +79,31 @@ jobs: needs: test permissions: contents: read + env: + RS_FILE_BASE: /tmp/rspace-filestore + services: + db: + image: mariadb:lts-jammy + env: + MARIADB_ROOT_PASSWORD: rspacedbpwd + MARIADB_DATABASE: rspace + MARIADB_USER: rspacedbuser + MARIADB_PASSWORD: rspacedbpwd + ports: + - 3306:3306 + options: >- + --tmpfs /var/lib/mysql:rw,noexec,nosuid,size=2g + --health-cmd="healthcheck.sh --connect --innodb_initialized" + --health-interval=5s + --health-timeout=5s + --health-retries=40 + --health-start-period=30s + # Required even though these are API-only tests: some backend startup + # paths check that the chemistry service is reachable. + chemistry: + image: rspaceops/oss-chemistry:latest + ports: + - 8090:8090 steps: - name: Checkout repository @@ -99,74 +124,105 @@ jobs: - name: Install dependencies run: poetry install --no-interaction - - name: Clone rspace-docker - run: git clone https://github.com/rspace-os/rspace-docker.git /tmp/rspace-docker - - - name: Download latest RSpace WAR + - name: Determine latest rspace-web release tag + id: rspace_web_release run: | set -euo pipefail - - release_json=$(curl -fsSL https://api.github.com/repos/rspace-os/rspace-web/releases/latest) - latest_tag=$(echo "$release_json" | jq -r '.tag_name') - war_url=$(echo "$release_json" | jq -r '.assets[]? | select(.name | test("^researchspace-.*\\.war$")) | .browser_download_url' | head -n1) - + latest_tag=$(curl -fsSL https://api.github.com/repos/rspace-os/rspace-web/releases/latest | jq -r '.tag_name') if [ -z "$latest_tag" ] || [ "$latest_tag" = "null" ]; then echo "Unable to determine latest release tag for rspace-web" exit 1 fi + echo "Latest rspace-web tag: $latest_tag" + echo "tag=$latest_tag" >> "$GITHUB_OUTPUT" - if [ -z "$war_url" ] || [ "$war_url" = "null" ]; then - war_url="https://github.com/rspace-os/rspace-web/releases/download/${latest_tag}/researchspace-${latest_tag}.war" - fi + - name: Checkout rspace-web + uses: actions/checkout@v4 + with: + repository: rspace-os/rspace-web + ref: ${{ steps.rspace_web_release.outputs.tag }} + path: rspace-web + persist-credentials: false - echo "Latest RSpace tag: $latest_tag" - curl -fsSL "$war_url" -o /tmp/rspace-docker/rspace.war + - name: Set up JDK + uses: actions/setup-java@v4 + with: + distribution: temurin + java-version: "17" + cache: maven + cache-dependency-path: rspace-web/pom.xml - - name: Free up disk space + - name: Configure MariaDB run: | - df -h - sudo rm -rf /usr/share/dotnet /usr/local/lib/android /opt/ghc /opt/hostedtoolcache/CodeQL || true - docker image prune -af || true - df -h - - - name: Start RSpace - working-directory: /tmp/rspace-docker - run: docker compose up -d - - - name: Wait for RSpace to be ready - timeout-minutes: 10 + mysql -h127.0.0.1 -uroot -prspacedbpwd <<'SQL' + SET GLOBAL character_set_server = 'utf8mb4'; + SET GLOBAL collation_server = 'utf8mb4_unicode_ci'; + SET GLOBAL sql_mode = 'STRICT_TRANS_TABLES,NO_ZERO_IN_DATE,NO_ZERO_DATE,ERROR_FOR_DIVISION_BY_ZERO,NO_ENGINE_SUBSTITUTION'; + GRANT CREATE, DROP ON *.* TO 'rspacedbuser'@'%'; + FLUSH PRIVILEGES; + SQL + + - name: Setup filestore + run: | + rm -rf "$RS_FILE_BASE" + mkdir -p "$RS_FILE_BASE" + + # Backend only - no -DgenerateReactDist, so no Node/pnpm/Vite build. + # The Python client only exercises the REST API, not the UI. + - name: Build RSpace backend + working-directory: rspace-web + run: ./mvnw clean package -DskipTests -Dtimestamp=${{ github.run_id }} -Djava-version=17 -Djava-vendor=temurin + + - name: Start RSpace and wait for readiness + working-directory: rspace-web + timeout-minutes: 15 run: | set -euo pipefail - - echo "Waiting for RSpace on http://localhost:8080 ..." - for i in {1..40}; do - if timeout 2 bash -c "cat < /dev/null > /dev/tcp/localhost/8080" 2>/dev/null; then - echo "RSpace is up" + nohup ./mvnw jetty:run-war \ + -DskipTests=true -Dmaven.war.skip=true \ + -Dtimestamp=${{ github.run_id }} \ + -Djava-version=17 -Djava-vendor=temurin \ + -Denvironment=drop-recreate-db \ + -Dspring.profiles.active=run \ + -DreactDevMode=false \ + -Dchemistry.provider=indigo \ + -Dchemistry.service.url=http://localhost:8090 \ + -Djdbc.url=jdbc:mysql://localhost:3306/rspace \ + -Djdbc.db.maven=rspace \ + -DRS_FILE_BASE="$RS_FILE_BASE" \ + > /tmp/jetty.log 2>&1 & + for i in $(seq 1 180); do + if [ "$(curl -s -o /dev/null -w '%{http_code}' http://localhost:8080/login || true)" = "200" ]; then + echo "Ready after ~$((i*5))s" exit 0 fi - echo " attempt $i/40..." - sleep 15 + sleep 5 done - echo "RSpace failed to start" + echo "RSpace did not become ready in time" + tail -200 /tmp/jetty.log exit 1 - - name: Install Playwright Browsers - run: poetry run python -m playwright install chromium --with-deps - - - name: Generate RSpace API Key - id: generate_key - run: poetry run python .github/scripts/get_rspace_api_key.py + # sysadmin1's account is a raw SQL fixture row - authenticating purely via + # its API key without ever logging in hits a lazy-initialization bug on + # its first write (home folder isn't created yet). One real login avoids + # it entirely by running the same initialization synchronously and correctly. + - name: Warm up sysadmin1 account + timeout-minutes: 5 + run: poetry run python .github/scripts/warmup_sysadmin.py env: RSPACE_URL: http://localhost:8080 - RSPACE_USERNAME: ${{ secrets.RSPACE_USERNAME }} - RSPACE_PASSWORD: ${{ secrets.RSPACE_PASSWORD }} - - - name: Mask API key - run: echo "::add-mask::${{ steps.generate_key.outputs.rspace_api_key }}" - name: Run integration tests timeout-minutes: 20 env: RSPACE_URL: http://localhost:8080 - RSPACE_API_KEY: ${{ steps.generate_key.outputs.rspace_api_key }} - run: poetry run pytest -m integration -v \ No newline at end of file + # Bootstrap sysadmin1 API key seeded by rspace-web's own "dev-test" + # Liquibase context (initial-seed-devtest.sql). Public, non-secret + # dev fixture value - not a credential that protects anything + # outside this ephemeral, job-local RSpace instance. + RSPACE_API_KEY: abcdefghijklmnop12 + run: poetry run pytest -m integration -v + + - name: Dump RSpace server log on failure + if: failure() + run: tail -300 /tmp/jetty.log || true \ No newline at end of file diff --git a/pyproject.toml b/pyproject.toml index f7325aa..ed05d22 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -31,7 +31,6 @@ setuptools = "<82" python-dotenv = "^1.1.1" black = "^21.6b0" pytest = "^8.0.0" -playwright = "^1.58.0" [build-system] requires = ["poetry-core>=1.0.0"] From b27ffbce229ed4585726b4ab11d102438e80db84 Mon Sep 17 00:00:00 2001 From: nebay-abraha Date: Tue, 7 Jul 2026 20:11:17 +0100 Subject: [PATCH 10/14] fixed Datacite connection testing to call the correct endpoints and validate the boolean response, and updated the related tests to match the corrected API behavior --- rspace_client/inv/inv.py | 103 ++++++++++++++++----------- rspace_client/tests/invapi_test.py | 108 +++++++++++++---------------- 2 files changed, 110 insertions(+), 101 deletions(-) diff --git a/rspace_client/inv/inv.py b/rspace_client/inv/inv.py index a857b54..8af595f 100644 --- a/rspace_client/inv/inv.py +++ b/rspace_client/inv/inv.py @@ -2655,87 +2655,106 @@ def barcode( fd.write(content) return content - def get_datacite_settings(self) -> dict: + @staticmethod + def _find_identifier_provider_settings(result: dict, provider: str) -> dict: + for group in result.get("identifiersSettings", {}).values(): + for entry in group: + if entry.get("provider") == provider: + return entry + raise ValueError(f"No settings found for provider '{provider}' in response: {result}") + + def get_datacite_settings(self, provider: str = "IGSN_DATACITE") -> dict: """ - Gets the current DataCite settings. + Gets the current settings for the given identifier provider. + + Parameters + ---------- + provider : str, optional + One of "IGSN_DATACITE", "PIDINST_DATACITE", "PIDINST_B2INST". Defaults to "IGSN_DATACITE". Returns ------- dict - The current DataCite settings + The current settings for the given provider """ - return self.retrieve_api_results( - "/system/settings", - request_type="GET" - ) + result = self.retrieve_api_results("/system/settings", request_type="GET") + return self._find_identifier_provider_settings(result, provider) - def update_datacite_settings(self, enabled: bool, server_url: str = None, username: str = None, password: str = None, repository_prefix: str = None) -> dict: + def update_datacite_settings( + self, + enabled: bool, + provider: str = "IGSN_DATACITE", + server_url: str = None, + username: str = None, + password: str = None, + repository_prefix: str = None, + ) -> dict: """ - Updates the DataCite settings. + Updates the settings for the given identifier provider. Parameters ---------- enabled : bool - Whether DataCite is enabled (True or False) + Whether this provider is enabled (True or False) + provider : str, optional + One of "IGSN_DATACITE", "PIDINST_DATACITE", "PIDINST_B2INST". Defaults to "IGSN_DATACITE". server_url : str, optional - DataCite server URL. Required when enabled=True + Server URL. Required when enabled=True username : str, optional - DataCite username. Required when enabled=True + Username. Required when enabled=True password : str, optional - DataCite password. Required when enabled=True + Password. Required when enabled=True repository_prefix : str, optional - DataCite repository prefix. Required when enabled=True + Repository prefix. Required when enabled=True Returns ------- dict - The updated settings + The updated settings for the given provider """ if enabled and (server_url is None or username is None or password is None or repository_prefix is None): raise ValueError("server_url, username, password, and repository_prefix are required when enabled=True") - - settings = { - "datacite": { - "enabled": str(enabled).lower() - } - } - - # Only include other settings if enabling DataCite - if enabled: - settings["datacite"].update({ - "serverUrl": server_url, - "username": username, - "password": password, - "repositoryPrefix": repository_prefix - }) - - return self.retrieve_api_results( + + body = {"provider": provider, "enabled": str(enabled).lower()} + if server_url is not None: + body["serverUrl"] = server_url + if username is not None: + body["username"] = username + if password is not None: + body["password"] = password + if repository_prefix is not None: + body["repositoryPrefix"] = repository_prefix + + result = self.retrieve_api_results( "/system/settings", request_type="PUT", - params=settings + params=body ) + return self._find_identifier_provider_settings(result, provider) - def test_datacite_connection(self) -> bool: + def test_datacite_connection(self, provider: str = "IGSN_DATACITE") -> bool: """ - Tests the connection to the configured DataCite server. + Tests the connection to the configured server for the given identifier provider. - This method calls the DataCite test connection endpoint to verify that: - - DataCite client is properly configured and initialized - - The connection to the DataCite API server can be established - - The stored credentials are valid + Parameters + ---------- + provider : str, optional + One of "IGSN_DATACITE", "PIDINST_DATACITE", "PIDINST_B2INST". Defaults to "IGSN_DATACITE". Returns ------- bool True if the connection test passes, False otherwise """ - - url = self._get_api_url() + "/identifiers/testDataCiteConnection" + endpoint_name = "testIgsnConnection" if provider == "IGSN_DATACITE" else "testPidinstConnection" + url = self._get_api_url() + f"/identifiers/{endpoint_name}" headers = self._get_headers("application/json") try: response = requests.get(url, headers=headers) - return response.status_code == 200 + if response.status_code != 200: + return False + return bool(response.json()) except requests.exceptions.RequestException: return False diff --git a/rspace_client/tests/invapi_test.py b/rspace_client/tests/invapi_test.py index 14b221b..8ec5fd8 100644 --- a/rspace_client/tests/invapi_test.py +++ b/rspace_client/tests/invapi_test.py @@ -998,28 +998,24 @@ def test_update_datacite_settings_enabled(self): # Verify the response structure self.assertIsInstance(result, dict) - self.assertIn("datacite", result) - datacite_settings = result["datacite"] - self.assertEqual(datacite_settings["serverUrl"], server_url) - self.assertEqual(datacite_settings["username"], username) - self.assertEqual(datacite_settings["password"], password) - self.assertEqual(datacite_settings["repositoryPrefix"], repository_prefix) - self.assertEqual(datacite_settings["enabled"], "true") + self.assertEqual(result["serverUrl"], server_url) + self.assertEqual(result["username"], username) + self.assertEqual(result["password"], password) + self.assertEqual(result["repositoryPrefix"], repository_prefix) + self.assertEqual(result["enabled"], "true") finally: # Restore original settings - if "datacite" in original_settings: - orig_datacite = original_settings["datacite"] - if orig_datacite.get("enabled") == "true": - self.invapi.update_datacite_settings( - enabled=True, - server_url=orig_datacite.get("serverUrl", ""), - username=orig_datacite.get("username", ""), - password=orig_datacite.get("password", ""), - repository_prefix=orig_datacite.get("repositoryPrefix", "") - ) - else: - self.invapi.update_datacite_settings(enabled=False) + if original_settings.get("enabled") == "true": + self.invapi.update_datacite_settings( + enabled=True, + server_url=original_settings.get("serverUrl", ""), + username=original_settings.get("username", ""), + password=original_settings.get("password", ""), + repository_prefix=original_settings.get("repositoryPrefix", "") + ) + else: + self.invapi.update_datacite_settings(enabled=False) def test_update_datacite_settings_disabled(self): """ @@ -1033,23 +1029,20 @@ def test_update_datacite_settings_disabled(self): result = self.invapi.update_datacite_settings(enabled=False) self.assertIsInstance(result, dict) - self.assertIn("datacite", result) - self.assertEqual(result["datacite"]["enabled"], "false") + self.assertEqual(result["enabled"], "false") finally: # Restore original settings - if "datacite" in original_settings: - orig_datacite = original_settings["datacite"] - if orig_datacite.get("enabled") == "true": - self.invapi.update_datacite_settings( - enabled=True, - server_url=orig_datacite.get("serverUrl", ""), - username=orig_datacite.get("username", ""), - password=orig_datacite.get("password", ""), - repository_prefix=orig_datacite.get("repositoryPrefix", "") - ) - else: - self.invapi.update_datacite_settings(enabled=False) + if original_settings.get("enabled") == "true": + self.invapi.update_datacite_settings( + enabled=True, + server_url=original_settings.get("serverUrl", ""), + username=original_settings.get("username", ""), + password=original_settings.get("password", ""), + repository_prefix=original_settings.get("repositoryPrefix", "") + ) + else: + self.invapi.update_datacite_settings(enabled=False) def test_datacite_connection_with_valid_settings(self): """ @@ -1078,18 +1071,16 @@ def test_datacite_connection_with_valid_settings(self): finally: # Restore original settings - if "datacite" in original_settings: - orig_datacite = original_settings["datacite"] - if orig_datacite.get("enabled") == "true": - self.invapi.update_datacite_settings( - enabled=True, - server_url=orig_datacite.get("serverUrl", ""), - username=orig_datacite.get("username", ""), - password=orig_datacite.get("password", ""), - repository_prefix=orig_datacite.get("repositoryPrefix", "") - ) - else: - self.invapi.update_datacite_settings(enabled=False) + if original_settings.get("enabled") == "true": + self.invapi.update_datacite_settings( + enabled=True, + server_url=original_settings.get("serverUrl", ""), + username=original_settings.get("username", ""), + password=original_settings.get("password", ""), + repository_prefix=original_settings.get("repositoryPrefix", "") + ) + else: + self.invapi.update_datacite_settings(enabled=False) def test_datacite_connection_with_invalid_settings(self): """ @@ -1116,18 +1107,16 @@ def test_datacite_connection_with_invalid_settings(self): finally: # Restore original settings - if "datacite" in original_settings: - orig_datacite = original_settings["datacite"] - if orig_datacite.get("enabled") == "true": - self.invapi.update_datacite_settings( - enabled=True, - server_url=orig_datacite.get("serverUrl", ""), - username=orig_datacite.get("username", ""), - password=orig_datacite.get("password", ""), - repository_prefix=orig_datacite.get("repositoryPrefix", "") - ) - else: - self.invapi.update_datacite_settings(enabled=False) + if original_settings.get("enabled") == "true": + self.invapi.update_datacite_settings( + enabled=True, + server_url=original_settings.get("serverUrl", ""), + username=original_settings.get("username", ""), + password=original_settings.get("password", ""), + repository_prefix=original_settings.get("repositoryPrefix", "") + ) + else: + self.invapi.update_datacite_settings(enabled=False) def test_get_datacite_settings(self): """ @@ -1137,5 +1126,6 @@ def test_get_datacite_settings(self): # Should return a dictionary self.assertIsInstance(result, dict) - # Should contain datacite section (even if empty/default) - self.assertIn("datacite", result) + # Should contain the provider's settings fields + self.assertIn("provider", result) + self.assertIn("enabled", result) From 274d30636859cef1a40fc483cfa7076b5f8fffe8 Mon Sep 17 00:00:00 2001 From: nebay-abraha Date: Tue, 7 Jul 2026 20:12:16 +0100 Subject: [PATCH 11/14] document how ci provisions rspace test instance --- DEVELOPING.md | 6 ++++++ README.md | 18 +++++++++++++++++- 2 files changed, 23 insertions(+), 1 deletion(-) diff --git a/DEVELOPING.md b/DEVELOPING.md index 062200f..bef4d7a 100644 --- a/DEVELOPING.md +++ b/DEVELOPING.md @@ -45,6 +45,12 @@ poetry run pytest -m integration ``` Integration tests should be run with a new RSpace account that does not belong to any groups. + +#### How CI runs integration tests + +CI doesn't use a long-lived RSpace deployment or your credentials. Each run builds `rspace-web` from source and starts it fresh (Maven/Jetty, seeded database). That gives a known built-in `sysadmin1` account and API key (seeded by rspace-web's own dev/test fixtures), but authenticating purely via API key without ever logging in hits a lazy-initialization bug on that account's first write (its home folder isn't created yet). So CI does one plain HTTP login first (`.github/scripts/warmup_sysadmin.py`), which runs the same initialization correctly, then uses the account's API key directly for the whole suite. See `.github/workflows/codeql-and-tests.yml`. + +If you want to reproduce this locally against your own from-source RSpace build (rather than any existing account), log in once as `sysadmin1` / `sysWisc23!` (e.g. run `warmup_sysadmin.py` against your instance) before pointing `RSPACE_API_KEY` at `abcdefghijklmnop12` in your `.env` - otherwise the first document-creation call will 500. ### Writing Tests diff --git a/README.md b/README.md index 63671a6..e74c5ad 100644 --- a/README.md +++ b/README.md @@ -18,7 +18,23 @@ For full details of our API specification, please see https:// +export RSPACE_API_KEY= +poetry run pytest -m integration +``` + +See [DEVELOPING.md](DEVELOPING.md) for more details. To install rspace-client and its dependencies, run From 19d2470a9434cd4ce1dfb90c4f49feec568daf65 Mon Sep 17 00:00:00 2001 From: nebay-abraha Date: Tue, 7 Jul 2026 20:33:35 +0100 Subject: [PATCH 12/14] fix instrument test --- rspace_client/tests/invapi_test.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/rspace_client/tests/invapi_test.py b/rspace_client/tests/invapi_test.py index 8ec5fd8..bbccc99 100644 --- a/rspace_client/tests/invapi_test.py +++ b/rspace_client/tests/invapi_test.py @@ -921,7 +921,7 @@ def test_rename_instrument(self): def test_set_image_instrument(self): instrument = self.invapi.create_instrument(base.random_string(5)) - file = base.get_datafile("AntibodySample150.png") + file = base.get_datafile("antibodySample150.png") with open(file, "rb") as f: updated_instrument = self.invapi.set_image(instrument, f) From 4a644110898172dd15fb8910eaa46944e22e785b Mon Sep 17 00:00:00 2001 From: nebay-abraha Date: Wed, 8 Jul 2026 12:44:26 +0100 Subject: [PATCH 13/14] address copilot pr review --- .github/workflows/codeql-and-tests.yml | 2 +- rspace_client/inv/inv.py | 4 ++-- rspace_client/tests/invapi_test.py | 20 ++++++++++++-------- 3 files changed, 15 insertions(+), 11 deletions(-) diff --git a/.github/workflows/codeql-and-tests.yml b/.github/workflows/codeql-and-tests.yml index 5864831..09003fa 100644 --- a/.github/workflows/codeql-and-tests.yml +++ b/.github/workflows/codeql-and-tests.yml @@ -128,7 +128,7 @@ jobs: id: rspace_web_release run: | set -euo pipefail - latest_tag=$(curl -fsSL https://api.github.com/repos/rspace-os/rspace-web/releases/latest | jq -r '.tag_name') + latest_tag=$(curl -fsSL --retry 3 --retry-delay 2 https://api.github.com/repos/rspace-os/rspace-web/releases/latest | jq -r '.tag_name') if [ -z "$latest_tag" ] || [ "$latest_tag" = "null" ]; then echo "Unable to determine latest release tag for rspace-web" exit 1 diff --git a/rspace_client/inv/inv.py b/rspace_client/inv/inv.py index 8af595f..594cfd0 100644 --- a/rspace_client/inv/inv.py +++ b/rspace_client/inv/inv.py @@ -2661,7 +2661,7 @@ def _find_identifier_provider_settings(result: dict, provider: str) -> dict: for entry in group: if entry.get("provider") == provider: return entry - raise ValueError(f"No settings found for provider '{provider}' in response: {result}") + raise ValueError(f"No settings found for provider '{provider}' in /system/settings response") def get_datacite_settings(self, provider: str = "IGSN_DATACITE") -> dict: """ @@ -2755,7 +2755,7 @@ def test_datacite_connection(self, provider: str = "IGSN_DATACITE") -> bool: if response.status_code != 200: return False return bool(response.json()) - except requests.exceptions.RequestException: + except (requests.exceptions.RequestException, ValueError): return False def _calculate_start_index( diff --git a/rspace_client/tests/invapi_test.py b/rspace_client/tests/invapi_test.py index bbccc99..80c56dc 100644 --- a/rspace_client/tests/invapi_test.py +++ b/rspace_client/tests/invapi_test.py @@ -691,14 +691,18 @@ def test_barcode(self): self.assertTrue(len(barcode_bytes) > 0) self.assertTrue(barcode_bytes.startswith(b"\x89PNG\r\n\x1a\n")) - qr_bytes = self.invapi.barcode( - "SA12345", outfile="out10.png", barcode_type=inv.BarcodeFormat.QR - ) - self.assertTrue(len(qr_bytes) > 0) - self.assertTrue(qr_bytes.startswith(b"\x89PNG\r\n\x1a\n")) - self.assertTrue(os.path.exists("out10.png")) - self.assertTrue(os.path.getsize("out10.png") > 0) - os.remove("out10.png") + outfile = "out10.png" + try: + qr_bytes = self.invapi.barcode( + "SA12345", outfile=outfile, barcode_type=inv.BarcodeFormat.QR + ) + self.assertTrue(len(qr_bytes) > 0) + self.assertTrue(qr_bytes.startswith(b"\x89PNG\r\n\x1a\n")) + self.assertTrue(os.path.exists(outfile)) + self.assertTrue(os.path.getsize(outfile) > 0) + finally: + if os.path.exists(outfile): + os.remove(outfile) def test_delete_samples(self): new_sample = self.invapi.create_sample("to_delete") From 9f9562d94d285d6ae144aa56d84e70bc0fe03914 Mon Sep 17 00:00:00 2001 From: nebay-abraha Date: Mon, 13 Jul 2026 00:32:25 +0100 Subject: [PATCH 14/14] update failing test --- .env.example | 1 + rspace_client/tests/invapi_test.py | 4 ++-- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/.env.example b/.env.example index 67ab854..a8297d8 100644 --- a/.env.example +++ b/.env.example @@ -1,3 +1,4 @@ RSPACE_URL= RSPACE_API_KEY= +RSPACE_SUPPORTS_LINK_FIELDS= diff --git a/rspace_client/tests/invapi_test.py b/rspace_client/tests/invapi_test.py index ca669bd..dd0d883 100644 --- a/rspace_client/tests/invapi_test.py +++ b/rspace_client/tests/invapi_test.py @@ -1134,8 +1134,8 @@ def test_get_datacite_settings(self): # Should contain the provider's settings fields self.assertIn("provider", result) self.assertIn("enabled", result) - # Should contain datacite section (even if empty/default) - self.assertIn("datacite", result) + self.assertIn("serverUrl", result) + self.assertIn("repositoryPrefix", result) def _require_link_field_support(self): """