Skip to content

Add test to ensure docs examples are valid cloud-init configs #355

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.

Sign up for GitHub

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

Merged
OddBloke merged 5 commits into from
May 8, 2020
Merged

Add test to ensure docs examples are valid cloud-init configs #355

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
87811c6
Add test to ensure docs examples are valid cloud-init configs
TheRealFalcon May 6, 2020
c7bbe26
[squash] review comments
TheRealFalcon May 7, 2020
f8639a9
[squash] parametrized test based on review comments
TheRealFalcon May 7, 2020
fbc9c0e
[squash] only creating the schema once
TheRealFalcon May 7, 2020
a3d0167
[squash] review comments
TheRealFalcon May 8, 2020
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
1 change: 1 addition & 0 deletions doc/examples/cloud-config-apt.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
#cloud-config
# apt_pipelining (configure Acquire::http::Pipeline-Depth)
# Default: disables HTTP pipelining. Certain web servers, such
# as S3 do not pipeline properly (LP: #948461).
Expand Down
2 changes: 2 additions & 0 deletions doc/examples/cloud-config-datasources.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
#cloud-config

# Documentation on data sources configuration options
datasource:
# Ec2
Expand Down
1 change: 1 addition & 0 deletions doc/examples/cloud-config-disk-setup.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
#cloud-config
# Cloud-init supports the creation of simple partition tables and file systems
# on devices.

Expand Down
1 change: 1 addition & 0 deletions doc/examples/cloud-config-landscape.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
#cloud-config
# Landscape-client configuration
#
# Anything under the top 'landscape: client' entry
Expand Down
1 change: 1 addition & 0 deletions doc/examples/cloud-config-rsyslog.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
#cloud-config
## the rsyslog module allows you to configure the systems syslog.
## configuration of syslog is under the top level cloud-config
## entry 'rsyslog'.
Expand Down
33 changes: 26 additions & 7 deletions tests/unittests/test_handler/test_schema.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
# This file is part of cloud-init. See LICENSE file for license information.

import cloudinit
from cloudinit.config.schema import (
CLOUD_CONFIG_HEADER, SchemaValidationError, annotated_cloudconfig_file,
get_schema_doc, get_schema, validate_cloudconfig_file,
Expand All @@ -12,6 +12,7 @@
import os
import pytest
from io import StringIO
from pathlib import Path
from textwrap import dedent
from yaml import safe_load

Expand Down Expand Up @@ -114,21 +115,20 @@ def test_validateconfig_schema_honors_formats(self):


class TestCloudConfigExamples:

SCHEMA = get_schema()
PARAMS = [
schema = get_schema()
params = [
(schema["id"], example)
for schema in SCHEMA["allOf"] for example in schema["examples"]]
for schema in schema["allOf"] for example in schema["examples"]]

@pytest.mark.parametrize("schema_id,example", PARAMS)
@pytest.mark.parametrize("schema_id,example", params)
@skipUnlessJsonSchema()
def test_validateconfig_schema_of_example(self, schema_id, example):
""" For a given example in a config module we test if it is valid
according to the unified schema of all config modules
"""
config_load = safe_load(example)
validate_cloudconfig_schema(
config_load, TestCloudConfigExamples.SCHEMA, strict=True)
config_load, self.schema, strict=True)


class ValidateCloudConfigFileTest(CiTestCase):
Expand Down Expand Up @@ -447,4 +447,23 @@ def test_all_integration_test_cloud_config_schema(self):
if errors:
raise AssertionError(', '.join(errors))


def _get_schema_doc_examples():
Comment thread
OddBloke marked this conversation as resolved.
examples_dir = Path(
cloudinit.__file__).parent.parent / 'doc' / 'examples'
assert examples_dir.is_dir()

all_text_files = (f for f in examples_dir.glob('cloud-config*.txt')
if not f.name.startswith('cloud-config-archive'))
return all_text_files


class TestSchemaDocExamples:
schema = get_schema()
Copy link
Copy Markdown
Collaborator

@OddBloke OddBloke May 7, 2020

Choose a reason for hiding this comment

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

This is a nit (so feel free to not address it), but TestCloudConfigExamples has SCHEMA in all-caps and referred to as TestCloudConfigExamples.SCHEMA in its test: it would be nice to be consistent.

Copy link
Copy Markdown
Contributor Author

@TheRealFalcon TheRealFalcon May 7, 2020

Choose a reason for hiding this comment

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

Would it be ok if I updated the other class? Class variables usually don't use all caps unless they're constants, and self is a less verbose way of accessing a class variable than typing out the whole class every time.

Copy link
Copy Markdown
Collaborator

@OddBloke OddBloke May 8, 2020

Choose a reason for hiding this comment

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

Would it be ok if I updated the other class?

In general, I expect us all to have ownership of all parts of the codebase, so the answer is yes. In this specific case, the code in question was added by @lucasmoura this week, so lets give him an opportunity to weigh in on what his thinking was.

Class variables usually don't use all caps unless they're constants

This is a constant, in the sense that we only expect to read from it, but I don't feel strongly about this either way.

self is a less verbose way of accessing a class variable than typing out the whole class every time.

Yep, I'd also be fine with this. I can see using the class name makes sense if we're really treating classes purely as namespaces (in which use self shouldn't have any special meaning), but I agree that it is an unusual spelling (and also makes renaming this class more work, for example).

Copy link
Copy Markdown
Contributor

@lucasmoura lucasmoura May 8, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

Sorry for the long delay, but I don't have any problem with the proposed change. Originally I thought about the schema as a constant, but I do agree that using self for that particular use case is less verbose


@pytest.mark.parametrize("example_path", _get_schema_doc_examples())
@skipUnlessJsonSchema()
def test_schema_doc_examples(self, example_path):
validate_cloudconfig_file(str(example_path), self.schema)

# vi: ts=4 expandtab syntax=python