-
Notifications
You must be signed in to change notification settings - Fork 3
113 Improve reporting of test failures for CQL tests, take two #159
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
base: master
Are you sure you want to change the base?
Changes from all commits
3f2efc5
e5dd14f
14e1d29
63e9150
8eff34c
055ac44
bbb13af
dc61fab
a3061d1
d4cd870
5073fed
be94399
2e8e9c9
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -16,6 +16,7 @@ | |
| # Functions to facilitate interactions with the underlying data store | ||
|
|
||
| import logging | ||
| from collections import OrderedDict | ||
| from itertools import tee | ||
|
|
||
| # Account for name change in itertools as of py3k | ||
|
|
@@ -59,6 +60,10 @@ def build_keyspace_objects(keyspaces, metadata): | |
| """Build a list of cassandra.metadata.KeyspaceMetadata objects from a list of strings and a c.m.Metadata instance. System keyspaces will be excluded.""" | ||
| all_keyspace_objs = [metadata.keyspaces[ks] for ks in keyspaces] if keyspaces is not None else metadata.keyspaces.values() | ||
|
|
||
| # Make sure tables are ordered (by table name) | ||
| for ks_obj in all_keyspace_objs: | ||
| ks_obj.tables = OrderedDict(sorted(ks_obj.tables.items(), key=lambda item: item[0])) | ||
|
|
||
|
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. A similar problem to what's discussed above: tables should also follow a coherent order. Tests on Python 2.7 were seeing tables returned in an apparently random order, which in turn caused comparisons to a reference output to fail only because of the ordering of elements within the output. |
||
| # Borrowed from itertools | ||
| def partition(pred, iterable): | ||
| t1, t2 = tee(iterable) | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -20,7 +20,7 @@ import tox | |
|
|
||
|
|
||
| # Default C* versions to include in all integration tests | ||
| DEFAULT_CASSANDRA_VERSIONS = ["2.1.22", "2.2.19", "3.0.23", "3.11.10", "4.0-rc1"] | ||
| DEFAULT_CASSANDRA_VERSIONS = ["2.1.22", "2.2.19", "3.0.23", "3.11.10", "4.0.0"] | ||
|
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. An update to 4.0.0... since I was already here :) |
||
|
|
||
| TOX_DEPENDENCIES = """pytest | ||
| subprocess32 ~= 3.5 | ||
|
|
@@ -33,21 +33,26 @@ def runCassandraContainer(client, version): | |
| return client.containers.run(name="adelphi", remove=True, detach=True, ports={9042: 9042}, image="cassandra:{}".format(version)) | ||
|
|
||
|
|
||
| def writeToxIni(version): | ||
| def writeToxIni(version, keep_tmpdirs = False): | ||
| config = configparser.ConfigParser() | ||
| config["tox"] = { "envlist": "py2, py3" } | ||
| envs = {"CASSANDRA_VERSION": version} | ||
| if keep_tmpdirs: | ||
| print("Preserving temporary directories") | ||
| envs["KEEP_LOGS"] = True | ||
| envStr = "\n ".join(["{} = {}".format(k,v) for (k,v) in envs.items()]) | ||
| config["testenv"] = {"deps": TOX_DEPENDENCIES, \ | ||
| "commands": "pytest {posargs}", \ | ||
| "setenv": "CASSANDRA_VERSION = {}".format(version)} | ||
| "setenv": envStr} | ||
| with open(TOX_CONFIG, 'w') as configfile: | ||
| config.write(configfile) | ||
|
|
||
| @click.command() | ||
| @click.option('--cassandra', '-c', multiple=True, type=str) | ||
| @click.option('--python', '-p', multiple=True, type=click.Choice(["py2","py3"], case_sensitive = False)) | ||
| @click.option("--pytest", "-t", type=str, help="Arguments to be passed to pytest") | ||
| def runtests(cassandra, python, pytest): | ||
| @click.option('--keep-tmpdirs', help="Preserve temporary directories", is_flag=True) | ||
| def runtests(cassandra, python, pytest, keep_tmpdirs): | ||
| client = docker.from_env() | ||
| tox_args = ["-e {}".format(py) for py in python] if python else [] | ||
| if pytest: | ||
|
|
@@ -68,7 +73,7 @@ def runtests(cassandra, python, pytest): | |
| try: | ||
| if os.path.exists(TOX_CONFIG): | ||
| os.remove(TOX_CONFIG) | ||
| writeToxIni(version) | ||
| writeToxIni(version, keep_tmpdirs) | ||
|
|
||
| # cmdline() will raise SystemExit when it's done so trap that here to avoid | ||
| # exiting all the things | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -62,6 +62,17 @@ def makeTempDirs(self): | |
| outputDir = os.path.join(base, "outputDir") | ||
| os.mkdir(outputDir) | ||
| self.dirs = TempDirs(base, outputDir) | ||
| self.addCleanup(self.cleanUpTempDirs) | ||
|
|
||
|
|
||
| def cleanUpTempDirs(self): | ||
| # TODO: Note that there's no easy way to access this from test-adelphi unless we modify the | ||
| # ini generation code... and I'm not completely sure that's worth it. Might want to think | ||
| # about just deleting this outright... or making it a CLI option that can be easily accessed. | ||
| if "KEEP_LOGS" in os.environ: | ||
| log.info("KEEP_LOGS env var set, preserving logs/output at {}".format(self.dirs.basePath)) | ||
| else: | ||
| shutil.rmtree(self.dirs.basePath) | ||
|
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Switched to cleanup functions rather than an explicit tearDown() when trying to diagnose a different problem but in the end I decided to keep the change. tearDown() does have some odd semantics; it's not called if setUp() doesn't succeed, for instance. Rather than worry about it in the future it seemed saner to just convert to cleanup methods directly. |
||
|
|
||
|
|
||
| def setUp(self): | ||
|
|
@@ -73,15 +84,3 @@ def setUp(self): | |
| log.info("Testing Cassandra version {}".format(self.version)) | ||
|
|
||
| self.makeTempDirs() | ||
|
|
||
|
|
||
| def tearDown(self): | ||
| super(SchemaTestCase, self).tearDown() | ||
|
|
||
| # TODO: Note that there's no easy way to access this from test-adelphi unless we modify the | ||
| # ini generation code... and I'm not completely sure that's worth it. Might want to think | ||
| # about just deleting this outright... or making it a CLI option that can be easily accessed. | ||
| if "KEEP_LOGS" in os.environ: | ||
| log.info("KEEP_LOGS env var set, preserving logs/output at {}".format(self.dirs.basePath)) | ||
| else: | ||
| shutil.rmtree(self.dirs.basePath) | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,6 +1,8 @@ | ||
| import difflib | ||
| import glob | ||
| import logging | ||
| import os | ||
| import re | ||
| import shutil | ||
| import sys | ||
|
|
||
|
|
@@ -15,25 +17,36 @@ | |
| import subprocess | ||
|
|
||
| from tests.integration import SchemaTestCase, setupSchema, getAllKeyspaces, dropNewKeyspaces | ||
| from tests.util.schemadiff import cqlDigestGenerator | ||
| from tests.util.schema_util import get_schema | ||
|
|
||
| log = logging.getLogger('adelphi') | ||
|
|
||
| CQL_REFERENCE_SCHEMA_PATH = "tests/integration/resources/cql-schemas/{}.cql" | ||
| CQL_REFERENCE_KS0_SCHEMA_PATH = "tests/integration/resources/cql-schemas/{}-ks0.cql" | ||
|
|
||
| def digestSet(schemaFile): | ||
| rv = set() | ||
| for (_, digest) in cqlDigestGenerator(schemaFile): | ||
| rv.add(digest) | ||
| return rv | ||
| KEYSPACE_LINE_REGEX = re.compile(r'\s*CREATE KEYSPACE IF NOT EXISTS (\w+) ') | ||
|
|
||
| def linesWithNewline(fpath): | ||
| if not os.path.exists(fpath): | ||
| print("File {} does not exist".format(fpath)) | ||
| if os.path.getsize(fpath) <= 0: | ||
| print("File {} is empty".format(fpath)) | ||
| print("Reading lines for file {}".format(fpath)) | ||
| with open(fpath) as f: | ||
| rv = f.readlines() | ||
| lastLine = rv[-1] | ||
| if not lastLine.endswith("\n"): | ||
| rv[-1] = lastLine + "\n" | ||
| return rv | ||
|
|
||
| def logCqlDigest(schemaFile, digestSet): | ||
| for (cql, digest) in cqlDigestGenerator(schemaFile): | ||
| if digest in digestSet: | ||
| log.info("Digest: {}, CQL: {}".format(digest,cql)) | ||
|
|
||
| def extractKeyspaceName(schemaPath): | ||
| with open(schemaPath) as schemaFile: | ||
| for line in schemaFile: | ||
| matcher = KEYSPACE_LINE_REGEX.match(line) | ||
| if matcher: | ||
| return matcher.group(1) | ||
| return None | ||
|
|
||
|
|
||
| class TestCql(SchemaTestCase): | ||
|
|
@@ -44,11 +57,7 @@ def setUp(self): | |
| self.origKeyspaces = getAllKeyspaces() | ||
| log.info("Creating schema") | ||
| setupSchema(self.buildSchema()) | ||
|
|
||
|
|
||
| def tearDown(self): | ||
| super(TestCql, self).tearDown() | ||
| dropNewKeyspaces(self.origKeyspaces) | ||
| self.addCleanup(dropNewKeyspaces, self.origKeyspaces) | ||
|
|
||
|
|
||
| # ========================== Helper functions ========================== | ||
|
|
@@ -64,19 +73,43 @@ def buildSchema(self): | |
|
|
||
|
|
||
| def compareToReferenceCql(self, referencePath, comparePath): | ||
| referenceSet = digestSet(referencePath) | ||
| compareSet = digestSet(comparePath) | ||
| compareLines = linesWithNewline(comparePath) | ||
| referenceLines = linesWithNewline(referencePath) | ||
|
|
||
| diffGen = difflib.unified_diff( | ||
| compareLines, | ||
| referenceLines, | ||
| fromfile=os.path.abspath(comparePath), | ||
| tofile=os.path.abspath(referencePath)) | ||
|
|
||
| diffEmpty = True | ||
| for line in diffGen: | ||
| if diffEmpty: | ||
| print("Diff of generated file ({}) against reference file ({})".format( | ||
| os.path.basename(comparePath), | ||
| os.path.basename(referencePath))) | ||
| diffEmpty = False | ||
| print(line.strip()) | ||
|
|
||
| refOnlySet = referenceSet - compareSet | ||
| if len(refOnlySet) > 0: | ||
| log.info("Statements in reference file {} but not in compare file {}:".format(referencePath, comparePath)) | ||
| logCqlDigest(referencePath, refOnlySet) | ||
| compareOnlySet = compareSet - referenceSet | ||
| if len(compareOnlySet) > 0: | ||
| log.info("Statements in compare file {} but not in reference file {}:".format(comparePath, referencePath)) | ||
| logCqlDigest(comparePath, compareOnlySet) | ||
| if not diffEmpty: | ||
| self.fail() | ||
|
|
||
| self.assertEqual(referenceSet, compareSet) | ||
|
|
||
| def combineSchemas(self): | ||
| outputDirPath = self.outputDirPath(self.version) | ||
| allOutputFileName = "{}-all".format(self.version) | ||
| allOutputPath = self.outputDirPath(allOutputFileName) | ||
|
|
||
| schemaPaths = glob.glob("{}/*/schema".format(outputDirPath)) | ||
| self.assertGreater(len(schemaPaths), 0) | ||
| schemas = { extractKeyspaceName(p) : p for p in schemaPaths} | ||
| sortedKeyspaces = sorted(schemas.keys()) | ||
|
|
||
| with open(allOutputPath, "w+") as allOutputFile: | ||
| cqlStr = "\n\n".join(open(schemas[ks]).read() for ks in sortedKeyspaces) | ||
| allOutputFile.write(cqlStr) | ||
|
|
||
| return allOutputPath | ||
|
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Another problem observed in testing: the ordering of individual keyspace schemas returned by the glob above isn't deterministic so there's no guarantee we'll get the right ordering of keyspaces when we reconstruct the "all" schema containing everything. To fix this we extract the keyspace name from each individual keyspace schema and traverse them in order when generating the "all" schema. Note that this assumes deterministic output of keyspace names as described above. |
||
|
|
||
|
|
||
| # ========================== Test functions ========================== | ||
|
|
@@ -87,7 +120,7 @@ def test_stdout(self): | |
|
|
||
| self.compareToReferenceCql( | ||
| CQL_REFERENCE_SCHEMA_PATH.format(self.version), | ||
| self.stdoutPath(self.version)) | ||
| stdoutPath) | ||
|
|
||
|
|
||
| def test_outputdir(self): | ||
|
|
@@ -96,24 +129,9 @@ def test_outputdir(self): | |
| os.mkdir(outputDirPath) | ||
| subprocess.run("adelphi --output-dir={} export-cql --no-metadata 2>> {}".format(outputDirPath, stderrPath), shell=True) | ||
|
|
||
| # Basic idea here is to find all schemas written to the output dir and aggregate them into a single schema | ||
| # file. We then compare this aggregated file to the reference schema. Ordering is important here but | ||
| # the current keyspace names hash to something that causes individual keyspaces to be discovered in the | ||
| # correct order. | ||
| outputDirPath = self.outputDirPath(self.version) | ||
| allOutputFileName = "{}-all".format(self.version) | ||
| allOutputPath = self.outputDirPath(allOutputFileName) | ||
|
|
||
| outputSchemas = glob.glob("{}/*/schema".format(outputDirPath)) | ||
| self.assertGreater(len(outputSchemas), 0) | ||
| with open(allOutputPath, "w+") as allOutputFile: | ||
| for outputSchema in outputSchemas: | ||
| with open(outputSchema) as outputSchemaFile: | ||
| shutil.copyfileobj(outputSchemaFile, allOutputFile) | ||
| allOutputFile.write("\n") | ||
| self.compareToReferenceCql( | ||
| CQL_REFERENCE_SCHEMA_PATH.format(self.version), | ||
| allOutputPath) | ||
| self.combineSchemas()) | ||
|
|
||
|
|
||
| def test_some_keyspaces_stdout(self): | ||
|
|
@@ -123,7 +141,7 @@ def test_some_keyspaces_stdout(self): | |
|
|
||
| self.compareToReferenceCql( | ||
| CQL_REFERENCE_KS0_SCHEMA_PATH.format(self.version), | ||
| self.stdoutPath(self.version)) | ||
| stdoutPath) | ||
|
|
||
|
|
||
| def test_some_keyspaces_outputdir(self): | ||
|
|
||
This file was deleted.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Keyspaces returned via export should be handled in an orderly way. This is essential for legit compares of prior output to what's done by the current code, which in turn is an essential feature for our integration tests.