Skip to content

Commit ae78d16

Browse files
committed
Somehow these tests have been passing but shouldn't have been
Signed-off-by: John McCrae <[email protected]>
1 parent 325d55a commit ae78d16

File tree

13 files changed

+274
-108
lines changed

13 files changed

+274
-108
lines changed

oc-chef-pedant/Gemfile

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,8 @@ gem "rest-client", git: "https://github.com/chef/rest-client", branch: "jfm/ucrt
1414
# gem "chef", "18.8.46" - commmenting this out to get past loading that enormous gem to only get a version number.
1515

1616
# For "rake chef_zero_spec"
17-
# gem 'chef-zero', github: 'chef/chef-zero'
17+
# PR #352 merged to main but not released yet - use main branch
18+
gem 'chef-zero', github: 'chef/chef-zero', branch: 'main'
1819

1920
# If you want to load debugging tools into the bundle exec sandbox,
2021
# # add these additional dependencies into Gemfile.local

oc-chef-pedant/Gemfile.lock

Lines changed: 7 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -37,15 +37,15 @@ GEM
3737
binding_of_caller (1.0.1)
3838
debug_inspector (>= 1.2.0)
3939
byebug (12.0.0)
40-
chef-utils (18.8.46)
40+
chef-utils (18.8.54)
4141
concurrent-ruby
4242
coderay (1.1.3)
4343
concurrent-ruby (1.3.5)
4444
debug_inspector (1.2.0)
45-
diff-lcs (1.5.1)
45+
diff-lcs (1.6.2)
4646
domain_name (0.6.20240107)
4747
erubis (2.7.0)
48-
ffi (1.16.3)
48+
ffi (1.17.2-x64-mingw-ucrt)
4949
ffi-win32-extensions (1.0.4)
5050
ffi
5151
http-accept (2.1.1)
@@ -60,8 +60,6 @@ GEM
6060
mixlib-authentication (3.0.10)
6161
mixlib-config (3.0.27)
6262
tomlrb
63-
mixlib-shellout (3.3.9)
64-
chef-utils
6563
mixlib-shellout (3.3.9-x64-mingw-ucrt)
6664
chef-utils
6765
ffi-win32-extensions (~> 1.0.3)
@@ -79,7 +77,7 @@ GEM
7977
binding_of_caller (~> 1.0)
8078
pry (~> 0.13)
8179
public_suffix (6.0.2)
82-
rake (13.3.0)
80+
rake (13.3.1)
8381
rspec (3.13.2)
8482
rspec-core (~> 3.13.0)
8583
rspec-expectations (~> 3.13.0)
@@ -89,24 +87,22 @@ GEM
8987
rspec-expectations (3.13.5)
9088
diff-lcs (>= 1.2.0, < 2.0)
9189
rspec-support (~> 3.13.0)
92-
rspec-mocks (3.13.6)
90+
rspec-mocks (3.13.7)
9391
diff-lcs (>= 1.2.0, < 2.0)
9492
rspec-support (~> 3.13.0)
9593
rspec-rerun (1.1.0)
9694
rspec (~> 3.0)
9795
rspec-support (3.13.6)
9896
rspec_junit_formatter (0.6.0)
9997
rspec-core (>= 2, < 4, != 2.12.0)
100-
tomlrb (1.3.0)
98+
tomlrb (2.0.3)
10199
uuidtools (2.2.0)
102100
win32-process (0.10.0)
103101
ffi (>= 1.0.0)
104102
wmi-lite (1.0.7)
105103

106104
PLATFORMS
107-
ruby
108105
x64-mingw-ucrt
109-
x86_64-linux
110106

111107
DEPENDENCIES
112108
oc-chef-pedant!
@@ -117,4 +113,4 @@ DEPENDENCIES
117113
rest-client!
118114

119115
BUNDLED WITH
120-
2.6.9
116+
2.7.2

oc-chef-pedant/Rakefile

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@ task :chef_zero_spec do
1515
end
1616

1717
def bundle_exec_with_chef(test_gem, commands)
18-
gem_path = Bundler.environment.specs[test_gem].first.full_gem_path
18+
gem_path = Bundler.load.specs[test_gem].first.full_gem_path
1919
@gemfile_path = File.join(gem_path, "Gemfile.#{CURRENT_GEM_NAME}-external-test")
2020
gemfile = File.open(@gemfile_path, "w")
2121
begin

oc-chef-pedant/TESTING_GUIDE.md

Lines changed: 152 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,152 @@
1+
# oc-chef-pedant Testing Guide
2+
3+
This guide standardizes how to run the oc-chef-pedant suite across environments (chef-server vs chef-zero) and documents recent helper scope changes, risk notes, and the validation matrix post ActiveSupport removal.
4+
5+
---
6+
7+
## 1. Supported Execution Modes
8+
9+
| Mode | Target | Purpose | Invocation Summary |
10+
|------|--------|---------|--------------------|
11+
| Server (Embedded) | Full Chef Server (dev VM / omnibus install) | Primary regression signal | `chef-server-ctl test` (inside VM) or `bin/oc-chef-pedant -c pedant_config.rb` |
12+
| chef-zero (Released Gem) | Upstream chef-zero gem matching Chef 18 baseline | Fast API contract smoke, isolate server-side regressions | Rake task / direct script (see below) |
13+
| chef-zero (Branch Override) | Branch removing legacy Chef::VERSION references | Validate removal of ActiveSupport & version coupling | Export `CHEF_VERSION` or Bundler override Gemfile |
14+
15+
> Ruby 3.1.7 is the reference version (Chef 18 parity). Avoid 3.4+ until broader compatibility is validated.
16+
17+
---
18+
19+
## 2. Quick Commands
20+
21+
### 2.1 Inside Dev VM (full server)
22+
23+
```bash
24+
chef-server-ctl test --all # Full pedant + internal org tests
25+
chef-server-ctl test # Default subset
26+
chef-server-ctl test --all --exclude-internal-orgs
27+
```
28+
29+
### 2.2 Local (without full server) using chef-zero
30+
31+
From `oc-chef-pedant/`:
32+
33+
```bash
34+
# Ensure bundle resolves to the released chef-zero
35+
bundle install
36+
rake chef_zero_spec
37+
```
38+
39+
### 2.3 Using Branch Override of chef-zero
40+
41+
Use a dedicated Gemfile (example: `scripts/bk_tests/chef_zero-Gemfile`) or add a block like:
42+
43+
```ruby
44+
# Gemfile.local snippet
45+
# gem 'chef-zero', git: 'https://github.com/chef/chef-zero', branch: 'remove-chef-version'
46+
```
47+
48+
Then:
49+
50+
```bash
51+
BUNDLE_GEMFILE=scripts/bk_tests/chef_zero-Gemfile bundle install
52+
BUNDLE_GEMFILE=scripts/bk_tests/chef_zero-Gemfile rake chef_zero_spec
53+
```
54+
55+
### 2.4 Focused Spec Samples
56+
57+
```bash
58+
bundle exec rspec spec/api/keys/user_keys_spec.rb:930-970
59+
bundle exec rspec spec/api/server_api_version_spec.rb
60+
```
61+
62+
---
63+
64+
## 3. Helper Scope Changes (IMPORTANT)
65+
66+
Removed class-level singleton helpers:
67+
68+
- `platform`
69+
- `api_url`
70+
71+
Rationale: They caused large clusters of WrongScope failures when indirectly invoked in `before(:all)` and polluted example group scope. Use patterns below:
72+
73+
| Context | Old (removed) | New (explicit) |
74+
|---------|---------------|----------------|
75+
| before(:all)/after(:all) | `platform.create_user(name)` | `Pedant::Config.pedant_platform.create_user(name)` |
76+
| Example / let / before(:each) | `platform` (still OK via instance helper) | `platform` |
77+
| Building URLs (group scope) | `api_url("/nodes")` | `Pedant::Config.pedant_platform.api_url("/nodes")` |
78+
79+
Instance helpers remain: `platform`, `api_url(path)` inside example scope only.
80+
81+
---
82+
83+
## 4. Validation Matrix
84+
85+
| Dimension | Set | Notes |
86+
|-----------|-----|-------|
87+
| Ruby | 3.1.7 | Canonical; matches Chef 18 runtime |
88+
| Platform Mode | Full Server, chef-zero (released), chef-zero (branch) | Branch mode only when validating coupling removal |
89+
| Spec Slice | Smoke (server_api_version_spec + a CRUD resource), Core (default), Full (`--all`) | Smoke runs should complete < 2 min on typical dev hardware |
90+
| Headers | `X-Chef-Version` hardcoded KNIFE_VERSION | No dynamic Chef::VERSION dependency |
91+
| API Versions | v0 (legacy), negotiation path (`/server_api_version`) | Ensure `server_api_version_spec` passes in all modes |
92+
93+
Minimum CI gates after recent changes:
94+
95+
1. Server Mode Core suite: PASS
96+
2. chef-zero Released Smoke suite: PASS (server_api_version + basic ACL or keys spec subset)
97+
3. Branch Override (only while feature branch active): Smoke suite PASS
98+
4. Full Server `--all` (nightly) PASS
99+
100+
---
101+
102+
## 5. Risk & Compatibility Notes
103+
104+
| Area | Risk | Mitigation |
105+
|------|------|------------|
106+
| Helper Scope Removal | Hidden reliance on class-level `platform` in `before(:all)` blocks causing NameError later | Grep for `before(:all)` + `platform.`; replace with explicit config reference |
107+
| ActiveSupport Removal | Missed implicit inflector / Hash key conversions | Added targeted replacements (`pedant/concern`, `stringify_keys` shim) & core suite green before broader matrix |
108+
| Version Decoupling | Tools expecting dynamic Chef::VERSION in headers | Hardcoded `KNIFE_VERSION` unchanged; document expectation |
109+
| chef-zero Branch Divergence | Drift from released gem causing false positives | Keep branch usage confined to feature PR validation; do not gate merges on branch-only passes |
110+
| Ruby Version Skew | Newer Ruby 3.3/3.4 differences (warning categories, RSpec timing) | Pin CI to 3.1.7; add optional future lane once core stabilized |
111+
112+
Rollback Strategy: Re-introducing class-level helpers is low complexity but discouraged; prefer updating any lingering specs. ActiveSupport dependency should not be restored—issues should be fixed by adding narrowly scoped utilities.
113+
114+
---
115+
116+
## 6. Troubleshooting Quick Table
117+
118+
| Symptom | Likely Cause | Action |
119+
|---------|--------------|--------|
120+
| 1000+ WrongScope / method missing `platform` failures | A `before(:all)` still calling instance helper | Replace with `Pedant::Config.pedant_platform` |
121+
| 401/403 explosion under chef-zero only | Org/user provisioning order or missing association | Re-run single spec with `--backtrace`, inspect platform creation logs |
122+
| All requests 500 in chef-zero mode | Wrong chef-zero branch / dependency mismatch | Confirm Gemfile lock; run `ruby -v`; re-bundle with released gem |
123+
| Version negotiation failures | Header mismatch / test expecting dynamic version | Inspect `X-Chef-Version` in captured request log |
124+
125+
---
126+
127+
## 7. Adding New Tests – Best Practices
128+
129+
- Use `let` for per-example dynamic data; use `shared(:symbol)` only when a value must be accessible inside `before(:all)`.
130+
- Avoid creating real users/orgs in `before(:all)` unless they are immutable and shared; prefer `before(:each)` for mutable objects.
131+
- Always clean up with explicit `Pedant::Config.pedant_platform.delete_*` calls in `after(:all)` for created actors.
132+
- For new endpoints: add smoke coverage to the Smoke slice (small curated set) to keep early failures tight.
133+
134+
---
135+
136+
## 8. Future Enhancements (Optional Backlog)
137+
138+
- Introduce a tagged `:smoke` subset with < 50 examples for <60s validation.
139+
- Add script to auto-rewrite lingering `platform.` calls in group hooks.
140+
- Parameterize KNIFE_VERSION via env var for experimental clients.
141+
142+
---
143+
144+
## 9. At-a-Glance Checklist (PR Author)
145+
146+
- [ ] Core suite passes locally (server mode)
147+
- [ ] Smoke suite passes with released chef-zero
148+
- [ ] No new `before(:all)` using instance helpers
149+
- [ ] Added/updated docs if new config flags introduced
150+
151+
---
152+
Last updated: 2025-11-06

oc-chef-pedant/lib/pedant/rspec/common.rb

Lines changed: 36 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,18 @@ module Common
3737
include Pedant::JSON
3838
include Pedant::Request
3939
include Pedant::RSpec::CommonResponses
40+
41+
# NOTE: Previously we defined class-level helpers `platform` and `api_url` so they
42+
# could be called from before(:all)/after(:all). This encouraged mixing example-
43+
# scoped state into group-level hooks and produced large waves of WrongScope style
44+
# failures under RSpec 3 when helpers were (indirectly) used during example group
45+
# definition. We intentionally remove those singleton methods; only instance-level
46+
# helpers remain. For before(:all)/after(:all) use
47+
# Pedant::Config.pedant_platform
48+
# or
49+
# Pedant::Config.pedant_platform.api_url("/path")
50+
# explicitly. This makes scope usage explicit and prevents accidental memoization
51+
# leakage across examples.
4052

4153

4254
# Request/Response
@@ -405,20 +417,22 @@ def once_per_run(key, &block)
405417
Pedant::OncePerRun.once_per_run(key, &block)
406418
end
407419

420+
# platform and api_url helper methods
421+
# These instance-level helpers are accessible from examples, let blocks,
422+
# before(:each), and after(:each). In before(:all)/after(:all) blocks, use
423+
# Pedant::Config.pedant_platform directly to avoid RSpec scope issues.
408424
def platform
409425
Pedant::Config.pedant_platform
410426
end
411427

428+
def api_url(path_fragment)
429+
Pedant::Config.pedant_platform.api_url(path_fragment)
430+
end
431+
412432
## TODO: Remove this method; we probably don't need to access it directly
413433
def server
414-
platform.server
415-
end
416-
417-
# construct a complete API URL based on the pre-configured server information
418-
def api_url(path_fragment)
419-
platform.api_url(path_fragment)
434+
platform
420435
end
421-
422436
# Given a response object, verify the HTTP status is in the
423437
# 200-ish success range and raise an error if it is not. This
424438
# is intended to be used in helper/util modules where we want
@@ -469,21 +483,21 @@ def set_opt(opt, default)
469483
################################################################################
470484

471485
# If these are referenced in before(:all) blocks, use shared() instead of let()
472-
shared(:organization) { platform.test_org }
473-
shared(:org) { platform.test_org.name }
486+
shared(:organization) { Pedant::Config.pedant_platform.test_org }
487+
shared(:org) { Pedant::Config.pedant_platform.test_org.name }
474488
# TODO look at how these are set up - is accurate?
475-
shared(:admin_user) { platform.admin_user }
476-
shared(:org_admin) { platform.admin_user }
477-
shared(:normal_user) { platform.non_admin_user }
489+
shared(:admin_user) { Pedant::Config.pedant_platform.admin_user }
490+
shared(:org_admin) { Pedant::Config.pedant_platform.admin_user }
491+
shared(:normal_user) { Pedant::Config.pedant_platform.non_admin_user }
478492

479-
shared(:outside_user) { platform.bad_user }
493+
shared(:outside_user) { Pedant::Config.pedant_platform.bad_user }
480494

481495
# TODO no such thing - eliminate tests referring to it!
482-
shared(:admin_client) { platform.admin_client }
496+
shared(:admin_client) { Pedant::Config.pedant_platform.admin_client }
483497
# TODO all non-validator clients are normal clients.
484-
shared(:normal_client) { platform.non_admin_client }
485-
shared(:outside_client) { platform.bad_client }
486-
shared(:validator_client) { platform.validator_client }
498+
shared(:normal_client) { Pedant::Config.pedant_platform.non_admin_client }
499+
shared(:outside_client) { Pedant::Config.pedant_platform.bad_client }
500+
shared(:validator_client) { Pedant::Config.pedant_platform.validator_client }
487501

488502
shared(:knife_admin) { admin_user }
489503
shared(:knife_user) { normal_user }
@@ -492,18 +506,18 @@ def set_opt(opt, default)
492506
# away, and all tasks that require its use become methods on the
493507
# Platform object
494508

495-
shared(:superuser) { platform.superuser }
496-
shared(:superuser_key) { platform.superuser_key }
497-
shared(:webui_key) { platform.webui_key }
509+
shared(:superuser) { Pedant::Config.pedant_platform.superuser }
510+
shared(:superuser_key) { Pedant::Config.pedant_platform.superuser_key }
511+
shared(:webui_key) { Pedant::Config.pedant_platform.webui_key }
498512

499513
# Given a requestor, create a new one with the same name, but
500514
# with the web UI's private key for 'impersonation' tests
501515
def impersonate(requestor_to_impersonate)
502-
Pedant::Requestor.new(requestor_to_impersonate.name, platform.webui_key)
516+
Pedant::Requestor.new(requestor_to_impersonate.name, Pedant::Config.pedant_platform.webui_key)
503517
end
504518

505519
# Need a well-formed yet invalid key for a requestor to test authentiction
506-
shared(:bogus_key) { platform.bogus_key }
520+
shared(:bogus_key) { Pedant::Config.pedant_platform.bogus_key }
507521
shared(:invalid_user) { Pedant::Requestor.new("invalid", bogus_key, bogus: true) }
508522

509523
################################################################################

oc-chef-pedant/spec/api/account/account_acl_spec.rb

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -17,11 +17,11 @@ def rand_id
1717

1818
before(:all) do
1919
@test_orgname2 = "test-org-#{rand_id}"
20-
platform.create_org(@test_orgname2) if Pedant.config[:org][:create_me]
20+
Pedant::Config.pedant_platform.create_org(@test_orgname2) if Pedant.config[:org][:create_me]
2121
end
2222

2323
after(:all) do
24-
platform.delete_org(@test_orgname2) if Pedant.config[:org][:create_me]
24+
Pedant::Config.pedant_platform.delete_org(@test_orgname2) if Pedant.config[:org][:create_me]
2525
end
2626

2727
context "/users/<name>/_acl endpoint" do
@@ -1283,11 +1283,11 @@ def rand_id
12831283

12841284
context "PUT /#{type}/<name>/_acl/#{permission}" do
12851285
before(:all) do
1286-
@client_with_dot = platform.create_client("test-client.#{rand_id}")
1286+
@client_with_dot = Pedant::Config.pedant_platform.create_client("test-client.#{rand_id}")
12871287
end
12881288

12891289
after(:all) do
1290-
platform.delete_client(@client_with_dot)
1290+
Pedant::Config.pedant_platform.delete_client(@client_with_dot)
12911291
end
12921292

12931293

oc-chef-pedant/spec/api/account/account_group_spec.rb

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -440,11 +440,11 @@
440440

441441
before(:all) do
442442
@test_orgname2 = "test-org-#{rand_id}"
443-
platform.create_org(@test_orgname2) if Pedant.config[:org][:create_me]
443+
Pedant::Config.pedant_platform.create_org(@test_orgname2) if Pedant.config[:org][:create_me]
444444
end
445445

446446
after(:all) do
447-
platform.delete_org(@test_orgname2) if Pedant.config[:org][:create_me]
447+
Pedant::Config.pedant_platform.delete_org(@test_orgname2) if Pedant.config[:org][:create_me]
448448
end
449449

450450
before :each do

0 commit comments

Comments
 (0)