shopdb-flask/tests/test_naming_gate_migrations.py

"""Holes in `scripts/check-naming-and-style.sh`, each held open by a test.

The naming gate must read the column names migrations CREATE, not just models.

`scripts/check-naming-and-style.sh` checked `db.Column` attribute names on
models and nothing else, while `migrations/versions/` is filtered out of the
file list every other check in that script uses. A column is born in a
migration, so from the day the gate was written no created column name was ever
looked at: a model attribute reading fine says nothing about what the database
actually got.

Two tests, and the second is the one that matters. A gate that prints nothing
looks identical whether it is clean or broken, and this one was silently
passing everything for its whole life, so the test plants a violation and
requires the gate to fail on it. Without that, a future edit that breaks the
regex would leave every test here green.
"""

import re
import shutil
import subprocess
from pathlib import Path

import pytest

REPO_ROOT = Path(__file__).resolve().parent.parent
GATE = REPO_ROOT / 'scripts' / 'check-naming-and-style.sh'


def _find_bash():
    """Locate a bash that can actually run a POSIX script.

    shutil.which('bash') is not enough on Windows. The WSL shim at
    System32\\bash.exe is on PATH by default, satisfies which(), and then fails
    every invocation with

        WSL (...) ERROR: CreateProcessCommon:818: execvpe(/bin/bash) failed:
        No such file or directory

    when no distribution is installed - because the path handed to it is a
    Windows path that means nothing inside the WSL namespace. That is the same
    trap CONTRIBUTING.md documents for running the gate by hand, and it is why
    these 13 tests were recorded as "environmental failures" rather than being
    fixed: they were reporting the shim, not the gate.

    Git Bash is preferred explicitly, then any candidate is proven by running a
    trivial script through it before being trusted.
    """
    candidates = []
    git_bash = shutil.which('git')
    if git_bash:
        # <git>/cmd/git.exe -> <git>/bin/bash.exe
        candidates.append(str(Path(git_bash).resolve().parent.parent / 'bin'
                              / 'bash.exe'))
    candidates.extend([
        r'C:\Program Files\Git\bin\bash.exe',
        r'C:\Program Files (x86)\Git\bin\bash.exe',
    ])
    found = shutil.which('bash')
    if found:
        candidates.append(found)

    for candidate in candidates:
        if not candidate or not Path(candidate).exists():
            continue
        try:
            probe = subprocess.run([candidate, '-c', 'exit 0'],
                                   capture_output=True, text=True, timeout=30)
        except (OSError, subprocess.SubprocessError):
            continue
        if probe.returncode == 0 and 'execvpe' not in (probe.stderr or ''):
            return candidate
    return None


BASH = _find_bash()
needs_bash = pytest.mark.skipif(BASH is None, reason='needs a working bash')

# Same shapes the gate matches: the name literal of a created column, positional
# or keyword, on the sa.Column( line or on the line after it, plus the target of
# a rename.
COLUMN_LITERAL = re.compile(r"""sa\.Column\(\s*['"]([^'"]+)['"]""", re.S)
COLUMN_KEYWORD = re.compile(
    r"""sa\.Column\(\s*name\s*=\s*['"]([^'"]+)['"]""", re.S)
RENAME_LITERAL = re.compile(r"""new_column_name\s*=\s*['"]([^'"]+)['"]""")

# THE RULE, not a proxy for it. The first version of this file asked whether the
# name contained an underscore, which is the same mistake the gate's own regex
# made: 'Monitor_Name' has one but was missed for its leading capital, and a
# CamelCase name with no underscore at all is just as wrong and has nothing to
# find. Lowercase concatenated means lowercase concatenated.
LOWERCASE_CONCATENATED = re.compile(r'[a-z][a-z0-9]*\Z')

PLANTED = '''"""Planted by the test suite. Never applied."""
import sqlalchemy as sa
from alembic import op

revision = 'plantedprobe'
down_revision = None


def upgrade():
    op.add_column('widgets', sa.Column('planted_column', sa.Integer(),
                                       nullable=True))


def downgrade():
    op.drop_column('widgets', 'planted_column')
'''

# ASSEMBLED FROM FRAGMENTS, like the gate's own patterns and
# tests/test_docs_publishable.py: written literally, a test for the site-literal
# rule is itself a site literal, and both the gate and the publication scrub
# would find it here.
PLANTED_SITE_HOST = 'tsg' 'wp00525' '.geaero' 'space.net'
PLANTED_AGENT = (
    '// Planted by the test suite.\n'
    'internal static class Probe\n'
    '{\n'
    '    public const string DefaultBaseUrl = "https://'
    + PLANTED_SITE_HOST + '/shopdb";\n'
    '}\n'
)


def _migration_files():
    found = list((REPO_ROOT / 'migrations' / 'versions').glob('*.py'))
    found += list(REPO_ROOT.glob('plugins/*/migrations/versions/*.py'))
    return sorted(found)


def test_there_are_migrations_to_check():
    """Guard the guard: a glob that matches nothing passes everything below."""
    assert _migration_files(), 'no migrations found - has the layout moved?'


def test_no_migration_creates_a_badly_named_column():
    offenders = []
    for path in _migration_files():
        text = path.read_text(encoding='utf-8')
        for pattern in (COLUMN_LITERAL, COLUMN_KEYWORD, RENAME_LITERAL):
            for name in pattern.findall(text):
                if not LOWERCASE_CONCATENATED.match(name):
                    offenders.append('{0}: {1}'.format(
                        path.relative_to(REPO_ROOT), name))
    assert not offenders, (
        'column names are lowercase concatenated (CONTRIBUTING.md):\n  '
        + '\n  '.join(offenders))


@needs_bash
def test_the_gate_fails_on_a_planted_migration_column(tmp_path):
    """Plant one, and require the gate to say so.

    Run against a throwaway tree rather than the repository, so the planted
    file cannot be left behind by a failing assert. tmp_path is outside any
    checkout, so the script's git file list is unavailable and it takes its
    own find fallback - which is the branch a developer without git metadata
    gets, and so is worth exercising.
    """
    versions = tmp_path / 'migrations' / 'versions'
    versions.mkdir(parents=True)
    gate = tmp_path / 'scripts' / 'check-naming-and-style.sh'
    gate.parent.mkdir(parents=True)
    shutil.copy(GATE, gate)

    planted = versions / 'zz01_planted.py'
    planted.write_text(PLANTED, encoding='utf-8')
    dirty = subprocess.run([BASH, str(gate)], cwd=str(tmp_path),
                           capture_output=True, text=True)
    assert dirty.returncode == 1, (
        'the gate passed a snake_case migration column:\n' + dirty.stdout)
    assert 'planted_column' in dirty.stdout

    planted.unlink()
    clean = subprocess.run([BASH, str(gate)], cwd=str(tmp_path),
                           capture_output=True, text=True)
    assert clean.returncode == 0, (
        'the gate fails with nothing planted:\n' + clean.stdout)


@needs_bash
def test_the_gate_fails_on_a_site_literal_in_the_agent(tmp_path):
    """agent/ is in the ADR-015 scan, and has to stay in it.

    The C# agent is compiled once and installed on every bay at every site. A
    site hostname, base URL or pin baked into a .cs file there ships to the next
    site inside a binary, where no reviewer reads it - and agent/ was not one of
    the directories the site-literal scan looked at.
    """
    gate = tmp_path / 'scripts' / 'check-naming-and-style.sh'
    gate.parent.mkdir(parents=True)
    shutil.copy(GATE, gate)
    agent = tmp_path / 'agent' / 'Probe'
    agent.mkdir(parents=True)

    planted = agent / 'Probe.cs'
    planted.write_text(PLANTED_AGENT, encoding='utf-8')
    dirty = subprocess.run([BASH, str(gate)], cwd=str(tmp_path),
                           capture_output=True, text=True)
    assert dirty.returncode == 1, (
        'the gate passed a site hostname in the agent:\n' + dirty.stdout)
    assert 'Probe.cs' in dirty.stdout

    planted.unlink()
    clean = subprocess.run([BASH, str(gate)], cwd=str(tmp_path),
                           capture_output=True, text=True)
    assert clean.returncode == 0, (
        'the gate fails with nothing planted:\n' + clean.stdout)


# Every shape a migration writes a column name in. The gate's first regex asked
# for ['"][a-z0-9]*_ inside the quotes, so only the first of these was ever
# caught: a leading capital, a keyword argument or a CamelCase name with no
# underscore all walked past it. One case per shape, named so a failure says
# which shape regressed.
PLANTED_COLUMN_SHAPES = {
    'positional_snake': (
        "    op.add_column('widgets', sa.Column('planted_column',\n"
        "                                       sa.Integer()))\n", 'planted_column'),
    'positional_leading_capital': (
        "    op.add_column('widgets', sa.Column('Monitor_Name',\n"
        "                                       sa.String(50)))\n", 'Monitor_Name'),
    'positional_camelcase_no_underscore': (
        "    op.add_column('widgets', sa.Column('MonitorName', sa.String(50)))\n",
        'MonitorName'),
    'keyword_name': (
        "    op.add_column('widgets', sa.Column(name='kw_form',\n"
        "                                       type_=sa.String()))\n", 'kw_form'),
    'wrapped_after_open_paren': (
        "    op.add_column('widgets', sa.Column(\n"
        "        'wrapped_name', sa.Integer()))\n", 'wrapped_name'),
    'alter_column_rename': (
        "    op.alter_column('widgets', 'oldname',\n"
        "                    new_column_name='New_Name')\n", 'New_Name'),
    'create_table_second_column': (
        "    op.create_table('t2', sa.Column('goodone', sa.Integer()),\n"
        "                    sa.Column('Bad_Two', sa.Integer()))\n", 'Bad_Two'),
    'batch_alter_table_add': (
        "    with op.batch_alter_table('widgets') as batch_op:\n"
        "        batch_op.add_column(sa.Column('batchCol', sa.Integer()))\n",
        'batchCol'),
    'batch_alter_table_rename': (
        "    with op.batch_alter_table('widgets') as batch_op:\n"
        "        batch_op.alter_column('a', new_column_name='b_c')\n", 'b_c'),
}

PLANTED_CLEAN_BODY = (
    "    op.add_column('widgets', sa.Column('goodname', sa.Integer()))\n"
    "    op.add_column('widgets', sa.Column(name='alsogood', type_=sa.String()))\n"
    "    op.alter_column('widgets', 'oldname', new_column_name='newname')\n"
    # drop_column names a column that already exists, so a bad name here is
    # deliberately NOT a finding: it was wrong where it was created, and
    # reporting it again makes one rename look like two.
    "    op.drop_column('widgets', 'legacy_old')\n"
)


def _migration_module(body):
    return ('"""Planted by the test suite. Never applied."""\n'
            'import sqlalchemy as sa\n'
            'from alembic import op\n\n'
            "revision = 'plantedprobe'\n"
            'down_revision = None\n\n\n'
            'def upgrade():\n' + body + '\n\n'
            'def downgrade():\n'
            '    pass\n')


def _gate_tree(tmp_path):
    gate = tmp_path / 'scripts' / 'check-naming-and-style.sh'
    gate.parent.mkdir(parents=True, exist_ok=True)
    shutil.copy(GATE, gate)
    return gate


@needs_bash
@pytest.mark.parametrize('shape', sorted(PLANTED_COLUMN_SHAPES))
def test_the_gate_catches_every_column_shape(tmp_path, shape):
    """Plant one shape at a time and require the gate to name it.

    Parametrised rather than one file holding all nine, so a regression says
    WHICH shape stopped being seen. A single file would fail on the first one
    and hide the rest.
    """
    body, expected = PLANTED_COLUMN_SHAPES[shape]
    versions = tmp_path / 'migrations' / 'versions'
    versions.mkdir(parents=True)
    gate = _gate_tree(tmp_path)

    planted = versions / 'zz01_planted.py'
    planted.write_text(_migration_module(body), encoding='utf-8')
    dirty = subprocess.run([BASH, str(gate)], cwd=str(tmp_path),
                           capture_output=True, text=True)
    assert dirty.returncode == 1, (
        'the gate passed a {0} column:\n{1}'.format(shape, dirty.stdout))
    assert expected in dirty.stdout, (
        'the gate failed but never named {0}:\n{1}'.format(
            expected, dirty.stdout))


@needs_bash
def test_the_gate_passes_well_named_columns(tmp_path):
    """The other half: a gate that fails on everything is no better.

    Includes a drop_column with a bad name, which is out of scope on purpose.
    """
    versions = tmp_path / 'migrations' / 'versions'
    versions.mkdir(parents=True)
    gate = _gate_tree(tmp_path)
    (versions / 'zz01_clean.py').write_text(
        _migration_module(PLANTED_CLEAN_BODY), encoding='utf-8')
    clean = subprocess.run([BASH, str(gate)], cwd=str(tmp_path),
                           capture_output=True, text=True)
    assert clean.returncode == 0, (
        'the gate failed on lowercase concatenated names:\n' + clean.stdout)


@needs_bash
def test_the_site_literal_scan_honours_its_include_list(tmp_path):
    """The ADR-015 scan must read the file types it declares, and no others.

    The scan carried --exclude=README.md AHEAD of its --include list, and on GNU
    grep a file-level exclude in that position cancels every include on the
    command. So the scan was reading EVERY file under agent/ and the other
    scanned trees, and --include='*.cs' was decorative - adding agent/ did not
    mean the C# agent was covered for the reason the flags claimed, and any
    later narrowing of the include list would have been silently inert.

    The .cs half is covered by the test above. This is the other half: a file
    type the scan does not declare must not fail it, which is only true when the
    include list is actually in force. Before the fix this run came back red on
    notes.txt.
    """
    gate = _gate_tree(tmp_path)
    agent = tmp_path / 'agent' / 'Probe'
    agent.mkdir(parents=True)
    for name in ('notes.txt', 'Probe.csproj', 'README.md'):
        (agent / name).write_text(
            'shipped to ' + PLANTED_SITE_HOST + '\n', encoding='utf-8')

    result = subprocess.run([BASH, str(gate)], cwd=str(tmp_path),
                            capture_output=True, text=True)
    assert result.returncode == 0, (
        'the scan read a file type it does not declare:\n' + result.stdout)