Skip to content

acceptance: forbid empty test.toml files#5297

Merged
pietern merged 2 commits into
mainfrom
no-empty-test-toml
May 21, 2026
Merged

acceptance: forbid empty test.toml files#5297
pietern merged 2 commits into
mainfrom
no-empty-test-toml

Conversation

@pietern
Copy link
Copy Markdown
Contributor

@pietern pietern commented May 21, 2026

Fail any acceptance test whose test.toml parses to an empty table (zero-byte or comment-only). The check lives in the acceptance test runner's config loader. Deletes 20 pre-existing offenders -- 4 zero-byte placeholders and 16 comment-only files. None of the comments in those files are relevant for the test.

This codifies a review comment that shows up from time to time and thereby removes ambiguity/overhead.

This pull request and its description were written by Isaac.

Adds tools/check_test_toml.py, wired into ./task checks, which fails if
any acceptance/**/test.toml parses to an empty table (zero-byte or
comment-only). Deletes 20 pre-existing offenders -- 4 zero-byte
placeholders and 16 comment-only files. None of the comments in those
files are relevant for the test.

Co-authored-by: Isaac
@pietern pietern requested a review from denik May 21, 2026 10:55
@pietern pietern temporarily deployed to test-trigger-is May 21, 2026 10:55 — with GitHub Actions Inactive
@pietern pietern temporarily deployed to test-trigger-is May 21, 2026 10:55 — with GitHub Actions Inactive
Copy link
Copy Markdown
Contributor

@denik denik left a comment

Choose a reason for hiding this comment

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

nit: personally I'd extend acceptance test runner to reject such configs instead of separate entry in taskfile.

Per review feedback: fail in DoLoadConfig when a test.toml has no
settings, instead of a separate ./task checks entry.

Co-authored-by: Isaac
@pietern pietern temporarily deployed to test-trigger-is May 21, 2026 12:55 — with GitHub Actions Inactive
@pietern pietern temporarily deployed to test-trigger-is May 21, 2026 12:55 — with GitHub Actions Inactive
Copy link
Copy Markdown
Contributor Author

@pietern pietern left a comment

Choose a reason for hiding this comment

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

@denik Good point, much cleaner.

@pietern pietern enabled auto-merge May 21, 2026 13:16
@pietern pietern added this pull request to the merge queue May 21, 2026
Merged via the queue into main with commit 07f272b May 21, 2026
28 checks passed
@pietern pietern deleted the no-empty-test-toml branch May 21, 2026 13:43
TanishqDatabricks pushed a commit to TanishqDatabricks/cli that referenced this pull request May 22, 2026
Fail any acceptance test whose `test.toml` parses to an empty table
(zero-byte or comment-only). The check lives in the acceptance test
runner's config loader. Deletes 20 pre-existing offenders -- 4 zero-byte
placeholders and 16 comment-only files. None of the comments in those
files are relevant for the test.

This codifies a review comment that shows up from time to time and
thereby removes ambiguity/overhead.

This pull request and its description were written by Isaac.
janniklasrose added a commit that referenced this pull request May 27, 2026
Picked up from the main merge:
- #5297 forbids empty test.toml files; delete the two comment-only ones.
- modernize: replace NumField loop with sdkType.Fields() iteration.
- testifylint: assert.Empty over assert.Equal(t, "", ...).

Co-authored-by: Isaac
denik pushed a commit that referenced this pull request May 28, 2026
Fail any acceptance test whose `test.toml` parses to an empty table
(zero-byte or comment-only). The check lives in the acceptance test
runner's config loader. Deletes 20 pre-existing offenders -- 4 zero-byte
placeholders and 16 comment-only files. None of the comments in those
files are relevant for the test.

This codifies a review comment that shows up from time to time and
thereby removes ambiguity/overhead.

This pull request and its description were written by Isaac.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants