From b9f84b960980806a7564949519b74b8c50202551 Mon Sep 17 00:00:00 2001 From: Alexander Bayandin Date: Tue, 8 Aug 2023 20:09:38 +0100 Subject: [PATCH] Improve test results format (#4549) ## Problem The current test history format is a bit inconvenient: - It stores all test results in one row, so all queries should include subqueries which expand the test - It includes duplicated test results if the rerun is triggered manually for one of the test jobs (for example, if we rerun `debug-pg14`, then the report will include duplicates for other build types/postgres versions) - It doesn't have a reference to run_id, which we use to create a link to allure report Here's the proposed new format: ``` id BIGSERIAL PRIMARY KEY, parent_suite TEXT NOT NULL, suite TEXT NOT NULL, name TEXT NOT NULL, status TEXT NOT NULL, started_at TIMESTAMPTZ NOT NULL, stopped_at TIMESTAMPTZ NOT NULL, duration INT NOT NULL, flaky BOOLEAN NOT NULL, build_type TEXT NOT NULL, pg_version INT NOT NULL, run_id BIGINT NOT NULL, run_attempt INT NOT NULL, reference TEXT NOT NULL, revision CHAR(40) NOT NULL, raw JSONB COMPRESSION lz4 NOT NULL, ``` ## Summary of changes - Misc allure changes: - Update allure to 2.23.1 - Delete files from previous runs in HTML report (by using `sync --delete` instead of `mv`) - Use `test-cases/*.json` instead of `suites.json`, using this directory allows us to catch all reruns. - Until we migrated `scripts/flaky_tests.py` and `scripts/benchmark_durations.py` store test results in 2 formats (in 2 different databases). --- .../actions/allure-report-generate/action.yml | 31 ++- .github/actions/download/action.yml | 2 +- .github/workflows/build_and_test.yml | 22 +- scripts/comment-test-report.js | 1 + .../ingest_regress_test_result-new-format.py | 198 ++++++++++++++++++ 5 files changed, 242 insertions(+), 12 deletions(-) create mode 100644 scripts/ingest_regress_test_result-new-format.py diff --git a/.github/actions/allure-report-generate/action.yml b/.github/actions/allure-report-generate/action.yml index a027de9464..daa369a1a0 100644 --- a/.github/actions/allure-report-generate/action.yml +++ b/.github/actions/allure-report-generate/action.yml @@ -2,6 +2,12 @@ name: 'Create Allure report' description: 'Generate Allure report from uploaded by actions/allure-report-store tests results' outputs: + base-url: + description: 'Base URL for Allure report' + value: ${{ steps.generate-report.outputs.base-url }} + base-s3-url: + description: 'Base S3 URL for Allure report' + value: ${{ steps.generate-report.outputs.base-s3-url }} report-url: description: 'Allure report URL' value: ${{ steps.generate-report.outputs.report-url }} @@ -63,8 +69,8 @@ runs: rm -f ${ALLURE_ZIP} fi env: - ALLURE_VERSION: 2.22.1 - ALLURE_ZIP_SHA256: fdc7a62d94b14c5e0bf25198ae1feded6b005fdbed864b4d3cb4e5e901720b0b + ALLURE_VERSION: 2.23.1 + ALLURE_ZIP_SHA256: 11141bfe727504b3fd80c0f9801eb317407fd0ac983ebb57e671f14bac4bcd86 # Potentially we could have several running build for the same key (for example, for the main branch), so we use improvised lock for this - name: Acquire lock @@ -102,6 +108,11 @@ runs: REPORT_PREFIX=reports/${BRANCH_OR_PR} RAW_PREFIX=reports-raw/${BRANCH_OR_PR}/${GITHUB_RUN_ID} + BASE_URL=https://${BUCKET}.s3.amazonaws.com/${REPORT_PREFIX}/${GITHUB_RUN_ID} + BASE_S3_URL=s3://${BUCKET}/${REPORT_PREFIX}/${GITHUB_RUN_ID} + REPORT_URL=${BASE_URL}/index.html + REPORT_JSON_URL=${BASE_URL}/data/suites.json + # Get previously uploaded data for this run ZSTD_NBTHREADS=0 @@ -110,10 +121,9 @@ runs: # There's no previously uploaded data for this $GITHUB_RUN_ID exit 0 fi - for S3_FILEPATH in ${S3_FILEPATHS}; do - time aws s3 cp --only-show-errors "s3://${BUCKET}/${S3_FILEPATH}" "${WORKDIR}" - archive=${WORKDIR}/$(basename $S3_FILEPATH) + time aws s3 cp --recursive --only-show-errors "s3://${BUCKET}/${RAW_PREFIX}/" "${WORKDIR}/" + for archive in $(find ${WORKDIR} -name "*.tar.zst"); do mkdir -p ${archive%.tar.zst} time tar -xf ${archive} -C ${archive%.tar.zst} rm -f ${archive} @@ -129,10 +139,9 @@ runs: sed -i 's| ${WORKDIR}/index.html @@ -144,8 +153,10 @@ runs: EOF time aws s3 cp --only-show-errors ${WORKDIR}/index.html "s3://${BUCKET}/${REPORT_PREFIX}/latest/index.html" - echo "report-url=${REPORT_URL}" >> $GITHUB_OUTPUT - echo "report-json-url=${REPORT_URL%/index.html}/data/suites.json" >> $GITHUB_OUTPUT + echo "base-url=${BASE_URL}" >> $GITHUB_OUTPUT + echo "base-s3-url=${BASE_S3_URL}" >> $GITHUB_OUTPUT + echo "report-url=${REPORT_URL}" >> $GITHUB_OUTPUT + echo "report-json-url=${REPORT_JSON_URL}" >> $GITHUB_OUTPUT echo "[Allure Report](${REPORT_URL})" >> ${GITHUB_STEP_SUMMARY} diff --git a/.github/actions/download/action.yml b/.github/actions/download/action.yml index d3f9bc0414..ce26e7825b 100644 --- a/.github/actions/download/action.yml +++ b/.github/actions/download/action.yml @@ -31,7 +31,7 @@ runs: BUCKET=neon-github-public-dev FILENAME=$(basename $ARCHIVE) - S3_KEY=$(aws s3api list-objects-v2 --bucket ${BUCKET} --prefix ${PREFIX%$GITHUB_RUN_ATTEMPT} | jq -r '.Contents[].Key' | grep ${FILENAME} | sort --version-sort | tail -1 || true) + S3_KEY=$(aws s3api list-objects-v2 --bucket ${BUCKET} --prefix ${PREFIX%$GITHUB_RUN_ATTEMPT} | jq -r '.Contents[]?.Key' | grep ${FILENAME} | sort --version-sort | tail -1 || true) if [ -z "${S3_KEY}" ]; then if [ "${SKIP_IF_DOES_NOT_EXIST}" = "true" ]; then echo 'SKIPPED=true' >> $GITHUB_OUTPUT diff --git a/.github/workflows/build_and_test.yml b/.github/workflows/build_and_test.yml index 27bad61639..eabe9d1ab4 100644 --- a/.github/workflows/build_and_test.yml +++ b/.github/workflows/build_and_test.yml @@ -471,6 +471,26 @@ jobs: --build-type ${BUILD_TYPE} \ --ingest suites.json + - name: Store Allure test stat in the DB (new) + if: ${{ !cancelled() && steps.create-allure-report.outputs.report-json-url }} + env: + COMMIT_SHA: ${{ github.event.pull_request.head.sha || github.sha }} + REPORT_JSON_URL: ${{ steps.create-allure-report.outputs.report-json-url }} + TEST_RESULT_CONNSTR: ${{ secrets.REGRESS_TEST_RESULT_CONNSTR_NEW }} + BASE_S3_URL: ${{ steps.create-allure-report.outputs.base-s3-url }} + run: | + aws s3 cp --only-show-errors --recursive ${BASE_S3_URL}/data/test-cases ./test-cases + + ./scripts/pysync + + export DATABASE_URL="$TEST_RESULT_CONNSTR" + poetry run python3 scripts/ingest_regress_test_result-new-format.py \ + --reference ${GITHUB_REF} \ + --revision ${COMMIT_SHA} \ + --run-id ${GITHUB_RUN_ID} \ + --run-attempt ${GITHUB_RUN_ATTEMPT} \ + --test-cases-dir ./test-cases + coverage-report: runs-on: [ self-hosted, gen3, small ] container: @@ -1067,7 +1087,7 @@ jobs: OLD_PREFIX=artifacts/${GITHUB_RUN_ID} FILENAME=neon-${{ runner.os }}-${build_type}-artifact.tar.zst - S3_KEY=$(aws s3api list-objects-v2 --bucket ${BUCKET} --prefix ${OLD_PREFIX} | jq -r '.Contents[].Key' | grep ${FILENAME} | sort --version-sort | tail -1 || true) + S3_KEY=$(aws s3api list-objects-v2 --bucket ${BUCKET} --prefix ${OLD_PREFIX} | jq -r '.Contents[]?.Key' | grep ${FILENAME} | sort --version-sort | tail -1 || true) if [ -z "${S3_KEY}" ]; then echo >&2 "Neither s3://${BUCKET}/${OLD_PREFIX}/${FILENAME} nor its version from previous attempts exist" exit 1 diff --git a/scripts/comment-test-report.js b/scripts/comment-test-report.js index b68df65c41..1410b8a0ca 100755 --- a/scripts/comment-test-report.js +++ b/scripts/comment-test-report.js @@ -223,6 +223,7 @@ module.exports = async ({ github, context, fetch, report }) => { } else { commentBody += `#### No tests were run or test report is not available\n` } + commentBody += autoupdateNotice let createCommentFn, listCommentsFn, updateCommentFn, issueNumberOrSha if (isPullRequest) { diff --git a/scripts/ingest_regress_test_result-new-format.py b/scripts/ingest_regress_test_result-new-format.py new file mode 100644 index 0000000000..cff1d9875f --- /dev/null +++ b/scripts/ingest_regress_test_result-new-format.py @@ -0,0 +1,198 @@ +#! /usr/bin/env python3 + +import argparse +import dataclasses +import json +import logging +import os +import re +import sys +from contextlib import contextmanager +from dataclasses import dataclass +from datetime import datetime, timezone +from pathlib import Path +from typing import Tuple + +import backoff +import psycopg2 +from psycopg2.extras import execute_values + +CREATE_TABLE = """ +CREATE TABLE IF NOT EXISTS results ( + id BIGSERIAL PRIMARY KEY, + parent_suite TEXT NOT NULL, + suite TEXT NOT NULL, + name TEXT NOT NULL, + status TEXT NOT NULL, + started_at TIMESTAMPTZ NOT NULL, + stopped_at TIMESTAMPTZ NOT NULL, + duration INT NOT NULL, + flaky BOOLEAN NOT NULL, + build_type TEXT NOT NULL, + pg_version INT NOT NULL, + run_id BIGINT NOT NULL, + run_attempt INT NOT NULL, + reference TEXT NOT NULL, + revision CHAR(40) NOT NULL, + raw JSONB COMPRESSION lz4 NOT NULL, + UNIQUE (parent_suite, suite, name, build_type, pg_version, started_at, stopped_at, run_id) +); +""" + + +@dataclass +class Row: + parent_suite: str + suite: str + name: str + status: str + started_at: datetime + stopped_at: datetime + duration: int + flaky: bool + build_type: str + pg_version: int + run_id: int + run_attempt: int + reference: str + revision: str + raw: str + + +TEST_NAME_RE = re.compile(r"[\[-](?Pdebug|release)-pg(?P\d+)[-\]]") + + +def err(msg): + print(f"error: {msg}") + sys.exit(1) + + +@contextmanager +def get_connection_cursor(connstr: str): + @backoff.on_exception(backoff.expo, psycopg2.OperationalError, max_time=150) + def connect(connstr): + conn = psycopg2.connect(connstr, connect_timeout=30) + conn.autocommit = True + return conn + + conn = connect(connstr) + try: + with conn.cursor() as cur: + yield cur + finally: + if conn is not None: + conn.close() + + +def create_table(cur): + cur.execute(CREATE_TABLE) + + +def parse_test_name(test_name: str) -> Tuple[str, int, str]: + build_type, pg_version = None, None + if match := TEST_NAME_RE.search(test_name): + found = match.groupdict() + build_type = found["build_type"] + pg_version = int(found["pg_version"]) + else: + # It's ok, we embed BUILD_TYPE and Postgres Version into the test name only for regress suite and do not for other suites (like performance) + build_type = "release" + pg_version = 14 + + unparametrized_name = re.sub(rf"{build_type}-pg{pg_version}-?", "", test_name).replace("[]", "") + + return build_type, pg_version, unparametrized_name + + +def ingest_test_result( + cur, + reference: str, + revision: str, + run_id: int, + run_attempt: int, + test_cases_dir: Path, +): + rows = [] + for f in test_cases_dir.glob("*.json"): + test = json.loads(f.read_text()) + # Drop unneded fields from raw data + raw = test.copy() + raw.pop("parameterValues") + raw.pop("labels") + raw.pop("extra") + + build_type, pg_version, unparametrized_name = parse_test_name(test["name"]) + labels = {label["name"]: label["value"] for label in test["labels"]} + row = Row( + parent_suite=labels["parentSuite"], + suite=labels["suite"], + name=unparametrized_name, + status=test["status"], + started_at=datetime.fromtimestamp(test["time"]["start"] / 1000, tz=timezone.utc), + stopped_at=datetime.fromtimestamp(test["time"]["stop"] / 1000, tz=timezone.utc), + duration=test["time"]["duration"], + flaky=test["flaky"] or test["retriesStatusChange"], + build_type=build_type, + pg_version=pg_version, + run_id=run_id, + run_attempt=run_attempt, + reference=reference, + revision=revision, + raw=json.dumps(raw), + ) + rows.append(dataclasses.astuple(row)) + + columns = ",".join(f.name for f in dataclasses.fields(Row)) + query = f"INSERT INTO results ({columns}) VALUES %s ON CONFLICT DO NOTHING" + execute_values(cur, query, rows) + + +def main(): + parser = argparse.ArgumentParser( + description="Regress test result uploader. \ + Database connection string should be provided via DATABASE_URL environment variable", + ) + parser.add_argument("--initdb", action="store_true", help="Initialuze database") + parser.add_argument( + "--reference", type=str, required=True, help="git reference, for example refs/heads/main" + ) + parser.add_argument("--revision", type=str, required=True, help="git revision") + parser.add_argument("--run-id", type=int, required=True, help="GitHub Workflow run id") + parser.add_argument( + "--run-attempt", type=int, required=True, help="GitHub Workflow run attempt" + ) + parser.add_argument( + "--test-cases-dir", + type=Path, + required=True, + help="Path to a dir with extended test cases data", + ) + + connstr = os.getenv("DATABASE_URL", "") + if not connstr: + err("DATABASE_URL environment variable is not set") + + args = parser.parse_args() + with get_connection_cursor(connstr) as cur: + if args.initdb: + create_table(cur) + + if not args.test_cases_dir.exists(): + err(f"test-cases dir {args.test_cases_dir} does not exist") + + if not args.test_cases_dir.is_dir(): + err(f"test-cases dir {args.test_cases_dir} it not a directory") + + ingest_test_result( + cur, + reference=args.reference, + revision=args.revision, + run_id=args.run_id, + run_attempt=args.run_attempt, + test_cases_dir=args.test_cases_dir, + ) + + +if __name__ == "__main__": + logging.getLogger("backoff").addHandler(logging.StreamHandler()) + main()