Skip to content

Validate test files against the schema they declare #1813

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
54 changes: 26 additions & 28 deletions source/unified-test-format/tests/Makefile
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The unified-tests.yml Github action runs both make and check_schema_version.sh, which seems redundant. What is the difference between those two scripts?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Um, yeah. check_schema_version.sh indeed handles the exact same thing we're doing here - with that in mind this PR definitely isn't necessary, but we should remove the double checks. I'll follow up on this.

Original file line number Diff line number Diff line change
@@ -1,5 +1,3 @@
SCHEMA=../schema-1.23.json

.PHONY: all \
atlas-data-lake \
auth \
Expand Down Expand Up @@ -62,91 +60,91 @@ all: atlas-data-lake \
# For specifications that contain multiple test folders, create a target for each folder
# in addition to a target for the specification itself
atlas-data-lake: HAS_AJV
@ajv test -s $(SCHEMA) -d "../../atlas-data-lake-testing/tests/unified/*.yml" --valid
@./check-files.sh valid ../../atlas-data-lake-testing/tests/unified/*.yml

auth: HAS_AJV
@ajv test -s $(SCHEMA) -d "../../auth/tests/unified/*.yml" --valid
@./check-files.sh valid ../../auth/tests/unified/*.yml

change-streams: HAS_AJV
@ajv test -s $(SCHEMA) -d "../../change-streams/tests/unified/*.yml" --valid
@./check-files.sh valid ../../change-streams/tests/unified/*.yml

client-side-encryption: HAS_AJV
@ajv test -s $(SCHEMA) -d "../../client-side-encryption/tests/unified/*.yml" --valid
@./check-files.sh valid ../../client-side-encryption/tests/unified/*.yml

client-side-operations-timeout: HAS_AJV
@ajv test -s $(SCHEMA) -d "../../client-side-operations-timeout/tests/*.yml" --valid
@./check-files.sh valid ../../client-side-operations-timeout/tests/*.yml

collection-management: HAS_AJV
@ajv test -s $(SCHEMA) -d "../../collection-management/tests/*.yml" --valid
@./check-files.sh valid ../../collection-management/tests/*.yml

command-logging-and-monitoring: command-logging-and-monitoring/logging command-logging-and-monitoring/monitoring

command-logging-and-monitoring/logging: HAS_AJV
@ajv test -s $(SCHEMA) -d "../../command-logging-and-monitoring/tests/logging/*.yml" --valid
@./check-files.sh valid ../../command-logging-and-monitoring/tests/logging/*.yml

command-logging-and-monitoring/monitoring: HAS_AJV
@ajv test -s $(SCHEMA) -d "../../command-logging-and-monitoring/tests/monitoring/*.yml" --valid
@./check-files.sh valid ../../command-logging-and-monitoring/tests/monitoring/*.yml

connection-monitoring-and-pooling: connection-monitoring-and-pooling/logging

connection-monitoring-and-pooling/logging: HAS_AJV
@ajv test -s $(SCHEMA) -d "../../connection-monitoring-and-pooling/tests/logging/*.yml" --valid
@./check-files.sh valid ../../connection-monitoring-and-pooling/tests/logging/*.yml

crud: HAS_AJV
@ajv test -s $(SCHEMA) -d "../../crud/tests/unified/*.yml" --valid
@./check-files.sh valid ../../crud/tests/unified/*.yml

gridfs: HAS_AJV
@ajv test -s $(SCHEMA) -d "../../gridfs/tests/*.yml" --valid
@./check-files.sh valid ../../gridfs/tests/*.yml

index-management: HAS_AJV
@ajv test -s $(SCHEMA) -d "../../index-management/tests/*.yml" --valid
@./check-files.sh valid ../../index-management/tests/*.yml

load-balancers: HAS_AJV
@ajv test -s $(SCHEMA) -d "../../load-balancers/tests/*.yml" --valid
@./check-files.sh valid ../../load-balancers/tests/*.yml

read-write-concern: HAS_AJV
@ajv test -s $(SCHEMA) -d "../../read-write-concern/tests/operation/*.yml" --valid
@./check-files.sh valid ../../read-write-concern/tests/operation/*.yml

retryable-reads: HAS_AJV
@ajv test -s $(SCHEMA) -d "../../retryable-reads/tests/unified/*.yml" --valid
@./check-files.sh valid ../../retryable-reads/tests/unified/*.yml

retryable-writes: HAS_AJV
@ajv test -s $(SCHEMA) -d "../../retryable-writes/tests/unified/*.yml" --valid
@./check-files.sh valid ../../retryable-writes/tests/unified/*.yml

run-command: HAS_AJV
@ajv test -s $(SCHEMA) -d "../../run-command/tests/unified/*.yml" --valid
@./check-files.sh valid ../../run-command/tests/unified/*.yml

server-discovery-and-monitoring: HAS_AJV
@ajv test -s $(SCHEMA) -d "../../server-discovery-and-monitoring/tests/unified/*.yml" --valid
@./check-files.sh valid ../../server-discovery-and-monitoring/tests/unified/*.yml

server-selection: server-selection/logging

server-selection/logging: HAS_AJV
@ajv test -s $(SCHEMA) -d "../../server-selection/tests/logging/*.yml" --valid
@./check-files.sh valid ../../server-selection/tests/logging/*.yml

sessions: HAS_AJV
@ajv test -s $(SCHEMA) -d "../../sessions/tests/*.yml" --valid
@./check-files.sh valid ../../sessions/tests/*.yml

transactions-convenient-api: HAS_AJV
@ajv test -s $(SCHEMA) -d "../../transactions-convenient-api/tests/unified/*.yml" --valid
@./check-files.sh valid ../../transactions-convenient-api/tests/unified/*.yml

transactions: HAS_AJV
@ajv test -s $(SCHEMA) -d "../../transactions/tests/unified/*.yml" --valid
@./check-files.sh valid ../../transactions/tests/unified/*.yml

unified-test-format: unified-test-format/invalid unified-test-format/valid-fail unified-test-format/valid-pass

unified-test-format/invalid: HAS_AJV
@# Redirect stdout to hide expected validation errors
@ajv test -s $(SCHEMA) -d "invalid/*.yml" --invalid > /dev/null && echo "invalid/*.yml passed test"
@./check-files.sh invalid invalid/*.yml > /dev/null && echo "invalid/*.yml passed test"

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is out of scope for this change, but maintaining an inclusive list of all test subdirectories seems dangerous because it can easily miss new specs or test directory changes. Can we switch to excluding the invalid test files and including all other test files in source/?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When I last refactored the Makefile, I made sure to add individual targets for ease of use. We could add a separate target that checks all YAML files, paired with logic to skip any YAML test file that doesn't include a schemaVersion parameter on the top level. That would leave only the invalid files to be skipped.

unified-test-format/valid-fail: HAS_AJV
@ajv test -s $(SCHEMA) -d "valid-fail/*.yml" --valid
@./check-files.sh valid valid-fail/*.yml

unified-test-format/valid-pass: HAS_AJV
@ajv test -s $(SCHEMA) -d "valid-pass/*.yml" --valid
@./check-files.sh valid valid-pass/*.yml

versioned-api: HAS_AJV
@ajv test -s $(SCHEMA) -d "../../versioned-api/tests/*.yml" --valid
@./check-files.sh valid ../../versioned-api/tests/*.yml

HAS_AJV:
@if ! command -v ajv > /dev/null; then \
Expand Down
33 changes: 33 additions & 0 deletions source/unified-test-format/tests/check-files.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
#!/usr/bin/env bash

if [[ $# -lt 2 ]]; then
echo "Usage: $0 <mode> files"
echo "Modes:"
echo " valid: Check that files are valid"
echo " invalid: Check that files are invalid"
exit 1
fi

mode=$1
shift

for file in "$@"; do
schema=$(grep -m 1 "^schemaVersion:" "${file}" | sed -E 's:^schemaVersion\: .*(1\.[0-9]+).*$:\1:')
minorSchemaVersion=$(echo "${schema}" | sed -E 's:1\.([0-9]+):\1:')
schemaFile="../schema-${schema}.json"

if [[ ! -f "${schemaFile}" ]]; then
echo "Warning: File ${file} specifies an invalid schema ${schema}, using latest instead"
schemaFile="../schema-latest.json"
# Latest always uses json-schema draft-2019-09
spec="draft2019"
elif [[ "${minorSchemaVersion}" -gt "23" ]]; then
# Starting with 1.24, the schema uses draft-2019-09
spec="draft2019"
else
# Versions up to 1.23 use draft-7
spec="draft7"
fi

ajv test --spec ${spec} -s "${schemaFile}" -d "${file}" --${mode}
done
Loading