chore(regression): remove dead test_advanced_config + Greenplum config gitignore#9949
Open
dpage wants to merge 1 commit into
Open
chore(regression): remove dead test_advanced_config + Greenplum config gitignore#9949dpage wants to merge 1 commit into
dpage wants to merge 1 commit into
Conversation
…g gitignore Two related bits of long-stale plumbing in web/regression/: * test_advanced_config.json.in — the README has told users for years to copy this template to test_advance_config.json (no "d") and customise it, but a repo-wide grep for either spelling shows zero code, test, or CI references. The .in template is dead, and the README/.gitignore also disagree about whether the copied filename has a "d" in it, which is a typo trap that has gone unnoticed precisely because nobody actually performs the copy step. * test_greenplum_config.json — Greenplum support was removed from pgAdmin years ago (only references left in the tree are historical release-notes entries from versions 1.4 through 4.12). Nothing reads the config any more. Drop the dead template, prune both stale lines from web/regression/.gitignore, and simplify the README to describe just the server-side test_config.json that the framework actually consumes. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
WalkthroughThis PR removes the advanced configuration template ( ChangesAdvanced Configuration Template Removal
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@web/regression/README.md`:
- Line 72: Update the sentence "expects to find the file in the directory" to
remove the double space between "the" and "directory" so it reads "expects to
find the file in the directory"; locate the string in web/regression/README.md
(the line containing "expects to find the file in the directory") and replace
the two spaces with a single space.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 8ca161db-2ed2-4313-92c9-36538e86f4d9
📒 Files selected for processing (3)
web/regression/.gitignoreweb/regression/README.mdweb/regression/test_advanced_config.json.in
💤 Files with no reviewable changes (2)
- web/regression/test_advanced_config.json.in
- web/regression/.gitignore
| 2b) After creating the server configuration file, add (or modify) | ||
| parameter values as per requirements. The pgAdmin4 regression framework | ||
| expects to find the files in the directory | ||
| expects to find the file in the directory |
There was a problem hiding this comment.
Fix the extra whitespace in the sentence.
Line 72 contains a double space: "expects to find the file in the directory" which appears to be a leftover from editing. The sentence should have a single space.
📝 Proposed fix
- expects to find the file in the directory
+ expects to find the file in the directory📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| expects to find the file in the directory | |
| expects to find the file in the directory |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@web/regression/README.md` at line 72, Update the sentence "expects to find
the file in the directory" to remove the double space between "the" and
"directory" so it reads "expects to find the file in the directory"; locate the
string in web/regression/README.md (the line containing "expects to find the
file in the directory") and replace the two spaces with a single space.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
web/regression/test_advanced_config.json.in— a 492-line template the README has instructed users to copy totest_advance_config.json(note: no "d") for years, but nothing in the test framework or CI reads either spelling of the resulting file. Repo-wide grep fortest_advance_config/test_advanced_configreturns hits only in this template, the README, and the local.gitignore— zero code references. The README and.gitignorealso disagreed about whether the copied filename has a "d" in it, which is a typo trap that has gone unnoticed precisely because nobody actually performs the copy step.test_greenplum_config.jsonentry fromweb/regression/.gitignore. Greenplum support was removed from pgAdmin years ago; the only remaining mentions in the tree are historical release-notes entries from 1.4–4.12.web/regression/README.mdsection 2 to describe just thetest_config.jsonthat the framework actually consumes.Net change: -511 / +8 across three files.
Test plan
grep -rnfortest_advance_config/test_advanced_configacross*.py,*.json,*.yml, Makefile, etc. — confirmed no code, test, or CI references.grep -rnforgreenplumacross the same — confirmed only historical release-notes hits.python regression/runtests.py --exclude feature_testsstill completes (2212 tests, only an unrelated pre-existing permissions assertion on a manually-created~/.pgadmindir).Summary by CodeRabbit
Documentation
Chores