Skip to content
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

ci: validate that schemas are up to date #389

Merged
merged 2 commits into from
Apr 19, 2024
Merged
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
5 changes: 4 additions & 1 deletion everyvoice/cli.py
Original file line number Diff line number Diff line change
Expand Up @@ -287,10 +287,13 @@ def update_schemas(
for filename, schema in SCHEMAS_TO_OUTPUT.items():
if (schema_dir_path / filename).exists():
raise FileExistsError(
f"Sorry a schema already exists for version {filename}. Please bump the minor version number and generate the schema again."
f"Sorry a schema already exists for version {filename}.\n"
"If it's already been published to the schema store, please bump the EveryVoice minor version number and generate the schemas again.\n"
"If the current minor version is still in development, just delete the schema files and try again."
)
with open(schema_dir_path / filename, "w") as f:
json.dump(schema.model_json_schema(), f, indent=2)
f.write("\n")


CLICK_APP = typer.main.get_group(app)
Expand Down
32 changes: 25 additions & 7 deletions everyvoice/tests/test_cli.py
Original file line number Diff line number Diff line change
Expand Up @@ -168,17 +168,11 @@ def test_command_help_messages(self):
self.assertEqual(result.exit_code, 0)

def test_update_schema(self):
result = self.runner.invoke(app, ["update-schemas"])
# The following two assertions will fail when we change the major or minor
# version of everyvoice. It's up to us to run `everyvoice update-schemas` in
# that case and commit the results, and these assertions here will remind us we
# forgot to do so.
self.assertNotEqual(result.exit_code, 0)
self.assertIn("FileExistsError", str(result))
dummy_contact = ContactInformation(
contact_name="Test Runner", contact_email="info@everyvoice.ca"
)
with tempfile.TemporaryDirectory() as tmpdir:
# Validate that schema generation works correctly.
_ = self.runner.invoke(app, ["update-schemas", "-o", tmpdir])
for filename, obj in SCHEMAS_TO_OUTPUT.items():
with open(Path(tmpdir) / filename, encoding="utf8") as f:
Expand All @@ -196,6 +190,30 @@ def test_update_schema(self):
)
)

# Make sure the generated schemas are identical to those saved in the repo,
# i.e., that we didn't change the models but forget to update the schemas.
for filename in SCHEMAS_TO_OUTPUT:
with open(Path(tmpdir) / filename, encoding="utf8") as f:
new_schema = f.read()
try:
with open(EV_DIR / ".schema" / filename, encoding="utf8") as f:
saved_schema = f.read()
except FileNotFoundError:
raise AssertionError(
f'Schema file {filename} is missing, please run "everyvoice update-schemas".'
)
self.assertEqual(
saved_schema,
new_schema,
'Schemas are out of date, please run "everyvoice update-schemas".',
Copy link
Member

Choose a reason for hiding this comment

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

great! nice and easy, this is all we need. Although maybe we should have a message that says that if the schemas change, we should bump the minor version number too?

Copy link
Member Author

@joanise joanise Apr 18, 2024

Choose a reason for hiding this comment

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

Hum, everyvoice update-schemas will already tell you that if you run it and the schemas exist. It seems redundant to say it again here.

)

# Next, but only if everything above passed, we make sure we can't overwrite
# existing schemas by accident.
result = self.runner.invoke(app, ["update-schemas"])
self.assertNotEqual(result.exit_code, 0)
self.assertIn("FileExistsError", str(result))

def test_inspect_checkpoint_help(self):
result = self.runner.invoke(app, ["inspect-checkpoint", "--help"])
self.assertIn("inspect-checkpoint [OPTIONS] MODEL_PATH", result.stdout)
Expand Down