feat(ext.generate): add optional features support for templates#768
feat(ext.generate): add optional features support for templates#768
Conversation
Allow .generate.yml templates to declare optional features with
conditional variables, exclude patterns, and ignore patterns based on
enabled/disabled state. Features support dependency resolution via
`requires` with automatic transitive cascading — disabled dependencies
skip prompting for dependent features entirely.
Also fix setup_template_items to catch ImportError and correct the
variables default from {} to [].
Related:
- Resolves Issue #743
📝 WalkthroughWalkthroughThis PR introduces feature-driven conditional template generation to Cement's generate extension. A new Changes
Sequence DiagramsequenceDiagram
participant User
participant Generator
participant Config as Template Config
participant FeatureProcessor
participant FileSystem
User->>Generator: generate(template, options)
Generator->>Config: load .generate.yml
Config-->>Generator: variables, features config
alt Features Defined
Generator->>FeatureProcessor: _process_features(features, vars)
FeatureProcessor->>FeatureProcessor: validate dependencies
loop For Each Feature
FeatureProcessor->>User: prompt for feature enablement
User-->>FeatureProcessor: feature choice
FeatureProcessor->>FeatureProcessor: check requires (dependencies)
alt Dependency Not Met
FeatureProcessor->>FeatureProcessor: auto-disable dependent features
end
FeatureProcessor->>FeatureProcessor: collect feature variables
end
FeatureProcessor->>FeatureProcessor: merge feature-enabled/disabled blocks
FeatureProcessor-->>Generator: processed vars, exclude_list, ignore_list
end
Generator->>FileSystem: render files based on feature state
FileSystem-->>User: generated project structure
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
Tip Try Coding Plans. Let us write the prompt for your AI agent so you can ship faster (with fewer bugs). 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: 6
🧹 Nitpick comments (2)
demo/generate-features/templates/generate/webapp/Dockerfile (1)
4-4: Consider narrowing the build context.Line 4 copies the entire project tree into the image. For a template, that makes it easy for generated apps to accidentally bake in local junk or secrets unless a
.dockerignoreis added. Copying only the required files, or shipping a.dockerignore, would keep the demo safer and leaner.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@demo/generate-features/templates/generate/webapp/Dockerfile` at line 4, The Dockerfile currently uses the broad "COPY . ." instruction which risks baking local junk/secrets into images; update the template by replacing the "COPY . ." step in the Dockerfile with an explicit list of COPY instructions that only add the application's build assets and manifest files (e.g., package.json, package-lock.json/yarn.lock, src/ or build/ folder as appropriate) and also include a template .dockerignore file in the template set that excludes node_modules, .git, .env, and other local artifacts—ensure the Dockerfile refers only to the files you explicitly copy and the .dockerignore is present in the generated project to prevent accidental inclusion.tests/data/templates/generate/test6/.generate.yml (1)
26-35: Makefeature2's disabled path explicit.
tests/data/templates/generate/test6/feature2-filereferencesfeature2_var, butfeature2_varis only added from the enabled block and this disabled block never ignores/excludes that file. In the defaulttest6path, the result depends on undefined-variable handling rather than the feature toggle itself. I'd either gatefeature2-filehere or assert the current disabled output explicitly in the tests.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/data/templates/generate/test6/.generate.yml` around lines 26 - 35, The disabled branch for the feature toggle "feature2" is missing an explicit exclusion for the template that references feature2_var, causing test output to depend on undefined-variable handling; update the "disabled.ignore" list to include the "feature2-file" pattern (e.g., add a glob matching feature2-file or '.*feature2-file.*') so that when feature2 is false the template is excluded, or alternatively add a disabled.variables entry providing a safe default for feature2_var; reference the "feature2", "feature2_var", and "feature2-file" symbols when making the change.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@cement/ext/ext_generate.py`:
- Around line 37-57: The current loop in ext_generate.py treats missing
prerequisite states as disabled because it checks requires inline; change this
by resolving dependencies before assigning feature_states: build a dependency
graph from features (use the existing 'features', 'feature_names' and each
feature's 'requires'), perform a topological sort or implement a recursive
resolver function (e.g., resolve_feature_state(feature_name) that consults and
sets feature_states) to compute each feature's enabled/disabled value, and then
replace the inline requires-checking block (the unsatisfied logic that uses
'requires', 'feature_states', 'self.app.pargs.defaults' and logs via
'self.app.log.warning') with calls to the resolver so chains like feature_c ->
feature_b -> feature_a are correctly resolved regardless of YAML order.
- Around line 223-226: The except block currently swallows all ImportError
instances; change it to catch AttributeError and ModuleNotFoundError as e, and
if e is a ModuleNotFoundError ensure e.name matches the requested module
identifier (variable _from) before handling it—otherwise re-raise so dependency
import failures propagate; keep the existing debug message that uses mod_name
and '.'.join(mod_parts) and call app.log.debug(msg) only when the missing module
is the requested one.
In `@demo/generate-features/README.md`:
- Around line 70-96: In demo/generate-features/README.md update the three fenced
code blocks that show the project tree outputs so each opening fence is ```text
instead of ``` (the blocks containing the Docker-enabled tree, the "With docker
disabled" tree, and the "With docker enabled, docker_compose disabled" tree);
this will satisfy markdownlint MD040 by explicitly marking the code blocks as
plain text.
In `@demo/generate-features/templates/generate/webapp/Dockerfile`:
- Around line 3-6: Create and switch to an unprivileged user in the Dockerfile:
add steps to create a non-root user (e.g., "appuser"), chown -R /app to transfer
ownership after COPY, and add a USER appuser line before the existing CMD
["python","app.py"] so the container drops root privileges when starting the
application; keep WORKDIR /app and COPY . . as-is but ensure the chown happens
after the copy.
In `@tests/ext/test_ext_generate.py`:
- Around line 233-299: The feature dependency resolver in _process_features() is
order-dependent because it computes `unsatisfied` from the partially built
`feature_states`, causing dependents declared before their requirements to be
auto-disabled; make resolution order-independent by computing feature enablement
via a fixpoint/graph traversal: build a map of feature -> requires list (from
the features input), validate unknown requirements, then iterate (or DFS) to
compute each feature's final enabled state using default values and transitive
requires until no changes occur (or use topological order/detect cycles),
replacing the current `unsatisfied` logic that reads partial `feature_states`;
also add a new test case (similar to existing tests) where a feature appears
before its requirement in the YAML (out-of-order) and assert the dependent is
enabled when its requirement defaults to enabled to prevent regressions.
- Around line 225-230: In _process_features(), replace the two assert statements
that validate a feature's missing 'name' and unknown 'requires' with explicit
exceptions (e.g., raise ValueError with the same descriptive messages such as
"Required feature config key missing: name" and the existing unknown-requires
message), and update the test test_generate_features_missing_name (and related
tests that expect AssertionError) to expect ValueError (or your chosen exception
type) instead of AssertionError so validation works even under python -O.
---
Nitpick comments:
In `@demo/generate-features/templates/generate/webapp/Dockerfile`:
- Line 4: The Dockerfile currently uses the broad "COPY . ." instruction which
risks baking local junk/secrets into images; update the template by replacing
the "COPY . ." step in the Dockerfile with an explicit list of COPY instructions
that only add the application's build assets and manifest files (e.g.,
package.json, package-lock.json/yarn.lock, src/ or build/ folder as appropriate)
and also include a template .dockerignore file in the template set that excludes
node_modules, .git, .env, and other local artifacts—ensure the Dockerfile refers
only to the files you explicitly copy and the .dockerignore is present in the
generated project to prevent accidental inclusion.
In `@tests/data/templates/generate/test6/.generate.yml`:
- Around line 26-35: The disabled branch for the feature toggle "feature2" is
missing an explicit exclusion for the template that references feature2_var,
causing test output to depend on undefined-variable handling; update the
"disabled.ignore" list to include the "feature2-file" pattern (e.g., add a glob
matching feature2-file or '.*feature2-file.*') so that when feature2 is false
the template is excluded, or alternatively add a disabled.variables entry
providing a safe default for feature2_var; reference the "feature2",
"feature2_var", and "feature2-file" symbols when making the change.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 135d9fa6-13c6-4365-b0b9-22206454922a
📒 Files selected for processing (45)
CHANGELOG.mdCLAUDE.mdcement/ext/ext_generate.pydemo/generate-features/README.mddemo/generate-features/templates/generate/webapp/.generate.ymldemo/generate-features/templates/generate/webapp/Dockerfiledemo/generate-features/templates/generate/webapp/README.mddemo/generate-features/templates/generate/webapp/app.pydemo/generate-features/templates/generate/webapp/docker-compose.ymltests/data/templates/generate/test10/.generate.ymltests/data/templates/generate/test10/take-metests/data/templates/generate/test11/.generate.ymltests/data/templates/generate/test11/feature1-onlytests/data/templates/generate/test11/feature2-onlytests/data/templates/generate/test11/take-metests/data/templates/generate/test12/.generate.ymltests/data/templates/generate/test12/take-metests/data/templates/generate/test13/.generate.ymltests/data/templates/generate/test13/take-metests/data/templates/generate/test14/.generate.ymltests/data/templates/generate/test14/feature1-onlytests/data/templates/generate/test14/feature2-onlytests/data/templates/generate/test14/feature3-onlytests/data/templates/generate/test14/take-metests/data/templates/generate/test6/.generate.ymltests/data/templates/generate/test6/feature1-filetests/data/templates/generate/test6/feature1-onlytests/data/templates/generate/test6/feature2-filetests/data/templates/generate/test6/feature2-onlytests/data/templates/generate/test6/no-feature1tests/data/templates/generate/test6/take-metests/data/templates/generate/test7/.generate.ymltests/data/templates/generate/test7/feature1-filetests/data/templates/generate/test7/feature1-onlytests/data/templates/generate/test7/feature2-filetests/data/templates/generate/test7/feature2-onlytests/data/templates/generate/test7/no-feature1tests/data/templates/generate/test7/take-metests/data/templates/generate/test8/.generate.ymltests/data/templates/generate/test8/take-metests/data/templates/generate/test9/.generate.ymltests/data/templates/generate/test9/feature1-onlytests/data/templates/generate/test9/feature2-onlytests/data/templates/generate/test9/take-metests/ext/test_ext_generate.py
| for feature in features: | ||
| assert feature.get('name') is not None, \ | ||
| "Required feature config key missing: name" | ||
|
|
||
| name = feature['name'] | ||
| default = feature.get('default', False) | ||
| requires = feature.get('requires', []) | ||
|
|
||
| # validate and check requires before prompting | ||
| for req in requires: | ||
| assert req in feature_names, \ | ||
| f"Feature '{name}' requires unknown feature '{req}'" | ||
|
|
||
| unsatisfied = [r for r in requires if not feature_states.get(r)] | ||
| if unsatisfied: | ||
| self.app.log.warning( | ||
| f"Feature '{name}' disabled (requires: {unsatisfied[0]})" | ||
| ) | ||
| feature_states[name] = False | ||
| elif self.app.pargs.defaults: | ||
| feature_states[name] = bool(default) |
There was a problem hiding this comment.
Resolve requires independently of declaration order.
Lines 50-57 currently treat "not resolved yet" the same as "disabled", so requires only works when prerequisites appear earlier in the YAML. A valid chain like feature_c -> feature_b -> feature_a is incorrectly disabled if the list is not already topologically sorted. Please resolve dependencies recursively or in a separate topological pass before deciding each feature state.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@cement/ext/ext_generate.py` around lines 37 - 57, The current loop in
ext_generate.py treats missing prerequisite states as disabled because it checks
requires inline; change this by resolving dependencies before assigning
feature_states: build a dependency graph from features (use the existing
'features', 'feature_names' and each feature's 'requires'), perform a
topological sort or implement a recursive resolver function (e.g.,
resolve_feature_state(feature_name) that consults and sets feature_states) to
compute each feature's enabled/disabled value, and then replace the inline
requires-checking block (the unsatisfied logic that uses 'requires',
'feature_states', 'self.app.pargs.defaults' and logs via 'self.app.log.warning')
with calls to the resolver so chains like feature_c -> feature_b -> feature_a
are correctly resolved regardless of YAML order.
| except (AttributeError, ImportError): | ||
| msg = 'unable to load template module ' + \ | ||
| f"'{mod_name}' from '{'.'.join(mod_parts)}'" | ||
| app.log.debug(msg) |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
In Python, when importing a module, how can code distinguish “the requested module does not exist” from “a dependency imported by that module failed to import”? Specifically, what is the difference between catching ImportError vs ModuleNotFoundError during module import?
💡 Result:
ModuleNotFoundError is a subclass of ImportError that is raised only when the import machinery cannot locate a module/package (i.e., nothing with that module name can be found on sys.path). [1]
ImportError is the broader “import failed” exception: it includes ModuleNotFoundError, but also covers other import-time failures such as from pkg import name when name doesn’t exist, loader problems, etc. (And importantly, the import statement can also propagate arbitrary exceptions raised while executing the module body—those won’t be ImportError at all.) [2]
Distinguishing “requested module missing” vs “dependency missing”
Catching ModuleNotFoundError alone is not sufficient, because it’s also what you get when the requested module exists but, during its import, it tries to import a missing dependency.
The standard way is: catch ModuleNotFoundError and check which module name was missing:
try:
import mypkg
except ModuleNotFoundError as e:
if e.name == "mypkg":
print("mypkg itself is not installed / not found")
else:
print(f"mypkg was found, but a dependency was missing: {e.name}")except ImportError:will catch both cases (and more), so it can accidentally hide real import-time problems you usually want to surface.except ModuleNotFoundError:lets you focus on the “some module couldn’t be located” case, ande.nametells you which module that was. [1]
Sources: [1] Python docs: ModuleNotFoundError is a subclass of ImportError raised when a module cannot be located. [2] CPython issue discussion noting that import/importlib.import_module can raise arbitrary exceptions from executing the module body.
🏁 Script executed:
cat -n cement/ext/ext_generate.py | sed -n '200,250p'Repository: datafolklabs/cement
Length of output: 2608
🏁 Script executed:
rg -n "__import__" cement/ext/ext_generate.py -B 5 -A 10Repository: datafolklabs/cement
Length of output: 847
🏁 Script executed:
# Search for app.__import__ definition or usage to understand what it does
rg -n "def __import__" cement/ -A 10Repository: datafolklabs/cement
Length of output: 926
Narrow ImportError to catch only when the requested module is missing.
The current code catches bare ImportError, which also catches ModuleNotFoundError for missing dependencies of the requested module—these import failures should propagate rather than be silently swallowed with only a debug log.
Instead, catch ModuleNotFoundError and check e.name to ensure the failure is actually for the requested module. This way, nested dependency failures and other import-time errors surface properly, preventing "generate" commands from silently disappearing.
Keep the AttributeError catch—it correctly handles the case where the module exists but the requested attribute doesn't.
except (AttributeError, ModuleNotFoundError) as e:
if isinstance(e, ModuleNotFoundError) and e.name != _from:
raise # Let dependency/other import errors propagate
msg = 'unable to load template module ' + \
f"'{mod_name}' from '{'.'.join(mod_parts)}'"
app.log.debug(msg)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@cement/ext/ext_generate.py` around lines 223 - 226, The except block
currently swallows all ImportError instances; change it to catch AttributeError
and ModuleNotFoundError as e, and if e is a ModuleNotFoundError ensure e.name
matches the requested module identifier (variable _from) before handling
it—otherwise re-raise so dependency import failures propagate; keep the existing
debug message that uses mod_name and '.'.join(mod_parts) and call
app.log.debug(msg) only when the missing module is the requested one.
| ``` | ||
| /tmp/myproject/ | ||
| ├── README.md | ||
| ├── app.py | ||
| ├── Dockerfile | ||
| └── docker-compose.yml | ||
| ``` | ||
|
|
||
| **With docker disabled** (docker_compose is auto-disabled via `requires`): | ||
|
|
||
| ``` | ||
| /tmp/myproject/ | ||
| ├── README.md | ||
| └── app.py | ||
| ``` | ||
|
|
||
| No Dockerfile, no docker-compose.yml — disabling docker automatically | ||
| disables docker_compose because it requires docker. | ||
|
|
||
| **With docker enabled, docker_compose disabled**: | ||
|
|
||
| ``` | ||
| /tmp/myproject/ | ||
| ├── README.md | ||
| ├── app.py | ||
| └── Dockerfile | ||
| ``` |
There was a problem hiding this comment.
Add language identifiers to the tree code fences.
The three fenced blocks starting at Line 70, Line 80, and Line 91 are missing a language tag, so markdownlint will keep flagging MD040 here. Using text is enough.
Suggested fix
-```
+```text
/tmp/myproject/
├── README.md
├── app.py
├── Dockerfile
└── docker-compose.yml@@
- +text
/tmp/myproject/
├── README.md
└── app.py
@@
-```
+```text
/tmp/myproject/
├── README.md
├── app.py
└── Dockerfile
</details>
<details>
<summary>🧰 Tools</summary>
<details>
<summary>🪛 markdownlint-cli2 (0.21.0)</summary>
[warning] 70-70: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
---
[warning] 80-80: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
---
[warning] 91-91: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
</details>
</details>
<details>
<summary>🤖 Prompt for AI Agents</summary>
Verify each finding against the current code and only fix it if needed.
In @demo/generate-features/README.md around lines 70 - 96, In
demo/generate-features/README.md update the three fenced code blocks that show
the project tree outputs so each opening fence is text instead of (the
blocks containing the Docker-enabled tree, the "With docker disabled" tree, and
the "With docker enabled, docker_compose disabled" tree); this will satisfy
markdownlint MD040 by explicitly marking the code blocks as plain text.
</details>
<!-- fingerprinting:phantom:poseidon:grasshopper -->
<!-- This is an auto-generated comment by CodeRabbit -->
| WORKDIR /app | ||
| COPY . . | ||
|
|
||
| CMD ["python", "app.py"] |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Check if the Dockerfile exists and read its full content
find demo -name "Dockerfile" -type f | head -20Repository: datafolklabs/cement
Length of output: 122
🏁 Script executed:
# Look for the specific file mentioned
cat demo/generate-features/templates/generate/webapp/DockerfileRepository: datafolklabs/cement
Length of output: 149
🏁 Script executed:
# Check for .dockerignore in the same directory
ls -la demo/generate-features/templates/generate/webapp/Repository: datafolklabs/cement
Length of output: 473
Drop root before starting the app.
The template never switches away from the default root user, so every generated container runs with full privileges. Add an unprivileged user and hand off /app before CMD.
Suggested fix
FROM python:{{ python_version }}-slim
WORKDIR /app
COPY . .
+RUN useradd --create-home --shell /usr/sbin/nologin appuser \
+ && chown -R appuser:appuser /app
+USER appuser
CMD ["python", "app.py"]📝 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.
| WORKDIR /app | |
| COPY . . | |
| CMD ["python", "app.py"] | |
| FROM python:{{ python_version }}-slim | |
| WORKDIR /app | |
| COPY . . | |
| RUN useradd --create-home --shell /usr/sbin/nologin appuser \ | |
| && chown -R appuser:appuser /app | |
| USER appuser | |
| CMD ["python", "app.py"] |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@demo/generate-features/templates/generate/webapp/Dockerfile` around lines 3 -
6, Create and switch to an unprivileged user in the Dockerfile: add steps to
create a non-root user (e.g., "appuser"), chown -R /app to transfer ownership
after COPY, and add a USER appuser line before the existing CMD
["python","app.py"] so the container drops root privileges when starting the
application; keep WORKDIR /app and COPY . . as-is but ensure the chown happens
after the copy.
| def test_generate_features_missing_name(tmp): | ||
| argv = ['generate', 'test8', tmp.dir, '--defaults'] | ||
|
|
||
| with GenerateApp(argv=argv) as app: | ||
| with raises(AssertionError, match='Required feature config key missing: name'): | ||
| app.run() |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify the feature-config validation path currently relies on Python `assert`
# and that these tests assert the same failure mode.
rg -n -C2 "assert .*feature|Required feature config key missing: name|requires unknown feature|AssertionError, match=.*feature" \
cement/ext/ext_generate.py tests/ext/test_ext_generate.pyRepository: datafolklabs/cement
Length of output: 5884
Replace assert-based validation with explicit exceptions.
Feature config validation in _process_features() uses Python assert statements (lines 38-39 and 47-48 in cement/ext/ext_generate.py) for both the missing-name check and the unknown-requires check. These assertions disappear when Python runs in optimized mode (python -O), causing invalid configurations to bypass validation silently instead of failing predictably. Replace assert statements with proper exception types (e.g., ValueError or a custom exception) and update these tests to expect the new exception type instead of AssertionError.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@tests/ext/test_ext_generate.py` around lines 225 - 230, In
_process_features(), replace the two assert statements that validate a feature's
missing 'name' and unknown 'requires' with explicit exceptions (e.g., raise
ValueError with the same descriptive messages such as "Required feature config
key missing: name" and the existing unknown-requires message), and update the
test test_generate_features_missing_name (and related tests that expect
AssertionError) to expect ValueError (or your chosen exception type) instead of
AssertionError so validation works even under python -O.
| def test_generate_features_requires_satisfied(tmp): | ||
| # test9: feature1=true (default), feature2=true (default), feature2 requires feature1 | ||
| argv = ['generate', 'test9', tmp.dir, '--defaults'] | ||
|
|
||
| with GenerateApp(argv=argv) as app: | ||
| app.run() | ||
|
|
||
| # both features enabled, both files present | ||
| assert exists_join(tmp.dir, 'take-me') | ||
| assert exists_join(tmp.dir, 'feature1-only') | ||
| assert exists_join(tmp.dir, 'feature2-only') | ||
|
|
||
|
|
||
| def test_generate_features_requires_not_satisfied(tmp): | ||
| # test11: feature1=false (default), feature2=true (default) but requires feature1 | ||
| # feature2 should be auto-disabled because feature1 is off | ||
| argv = ['generate', 'test11', tmp.dir, '--defaults'] | ||
|
|
||
| with GenerateApp(argv=argv) as app: | ||
| app.run() | ||
|
|
||
| assert exists_join(tmp.dir, 'take-me') | ||
|
|
||
| # feature1 disabled: feature1-only ignored | ||
| assert not exists_join(tmp.dir, 'feature1-only') | ||
|
|
||
| # feature2 auto-disabled via requires: feature2-only ignored | ||
| assert not exists_join(tmp.dir, 'feature2-only') | ||
|
|
||
|
|
||
| def test_generate_features_requires_unknown(tmp): | ||
| # test10: feature requires a nonexistent feature | ||
| argv = ['generate', 'test10', tmp.dir, '--defaults'] | ||
|
|
||
| with GenerateApp(argv=argv) as app: | ||
| with raises(AssertionError, match="requires unknown feature"): | ||
| app.run() | ||
|
|
||
|
|
||
| def test_generate_features_minimal(tmp): | ||
| # test13: feature with only name and default, no enabled/disabled blocks | ||
| argv = ['generate', 'test13', tmp.dir, '--defaults'] | ||
|
|
||
| with GenerateApp(argv=argv) as app: | ||
| app.run() | ||
|
|
||
| assert exists_join(tmp.dir, 'take-me') | ||
| with open(os.path.join(tmp.dir, 'take-me'), 'r') as f: | ||
| res = f.read() | ||
| assert 'myapp' in res | ||
|
|
||
|
|
||
| def test_generate_features_transitive_requires(tmp): | ||
| # test14: feature1=false, feature2=true requires feature1, | ||
| # feature3=true requires feature2 | ||
| # disabling feature1 should cascade: feature2 disabled, then feature3 disabled | ||
| argv = ['generate', 'test14', tmp.dir, '--defaults'] | ||
|
|
||
| with GenerateApp(argv=argv) as app: | ||
| app.run() | ||
|
|
||
| assert exists_join(tmp.dir, 'take-me') | ||
|
|
||
| # all three features should be disabled via transitive requires | ||
| assert not exists_join(tmp.dir, 'feature1-only') | ||
| assert not exists_join(tmp.dir, 'feature2-only') | ||
| assert not exists_join(tmp.dir, 'feature3-only') |
There was a problem hiding this comment.
Dependency resolution is still order-sensitive.
The new requires coverage only uses fixtures where prerequisites are declared earlier in YAML. _process_features() resolves unsatisfied from the partially built feature_states map, so a dependent that appears before its requirement will be auto-disabled incorrectly even if the requirement defaults to enabled. Please add an out-of-order regression case and make the resolver order-independent.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@tests/ext/test_ext_generate.py` around lines 233 - 299, The feature
dependency resolver in _process_features() is order-dependent because it
computes `unsatisfied` from the partially built `feature_states`, causing
dependents declared before their requirements to be auto-disabled; make
resolution order-independent by computing feature enablement via a
fixpoint/graph traversal: build a map of feature -> requires list (from the
features input), validate unknown requirements, then iterate (or DFS) to compute
each feature's final enabled state using default values and transitive requires
until no changes occur (or use topological order/detect cycles), replacing the
current `unsatisfied` logic that reads partial `feature_states`; also add a new
test case (similar to existing tests) where a feature appears before its
requirement in the YAML (out-of-order) and assert the dependent is enabled when
its requirement defaults to enabled to prevent regressions.
1 participant
Allow .generate.yml templates to declare optional features with conditional variables, exclude patterns, and ignore patterns based on enabled/disabled state. Features support dependency resolution via
requireswith automatic transitive cascading — disabled dependencies skip prompting for dependent features entirely.Also fix setup_template_items to catch ImportError and correct the variables default from {} to [].
Related:
Summary by CodeRabbit
New Features
Bug Fixes
Documentation
Tests