-
Notifications
You must be signed in to change notification settings - Fork 23
Refactor/add rust test for links #545
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
WalkthroughRemoves one entry-type test, adds a large link-type test module and a coordinator zome Handlebars test template, tweaks link-type scaffold no_spec filtering logic, and updates the web-app package.json test script to run Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 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 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@src/templates/link_type.rs`:
- Around line 80-83: The retain predicate in the no_spec branch is wrong: using
k != "tests" || k != "dnas" is always true so nothing is removed. Change the
predicate to exclude both "tests" and "dnas" (e.g. k != "tests" && k != "dnas"
or !(k == "tests" || k == "dnas")) inside the closure passed to
link_type_template.dir_content_mut().map(|v| { v.retain(...); v }) so that
entries with keys "tests" or "dnas" are actually filtered out when no_spec is
true.
In `@src/templates/link_type/tests.rs`:
- Around line 113-118: The test is looking for generated test filenames using
kebab-case "-to-" (in the PathBuf format string inside the assertion that checks
scaffolded_template.file_tree via file_exists), but the template emits
snake_case "_to_" names; update the assertion(s) that build the expected path
(the string formatting using from_role and to_role_plural that currently
contains "-to-") to use "_to_" (snake_case) or normalize both sides
consistently; specifically change the path segment
"tests/{from_role}-to-{to_role_plural}.rs" to match the template's
"tests/{from_role}_to_{to_role_plural}.rs" for the assertions that reference
pluralize, to_role_plural, file_exists and scaffolded_template.file_tree.
36a5742 to
37f7f2a
Compare
37f7f2a to
dba41e2
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@src/templates/link_type/tests.rs`:
- Around line 131-159: The generated tests hardcode a hyphenated zome identifier
which produces invalid Rust; update the test_header function to use a
snake_cased zome name and update the Handlebars template usage of
coordinator_zome_manifest.name to apply the snake_case helper. Specifically, in
the Rust generator function test_header replace the hardcoded "test-zome"
occurrences with the snake-cased zome identifier used by the template, and in
the Handlebars template change any use of {{coordinator_zome_manifest.name}}
(and any places passed to zome(...)) to {{snake_case
coordinator_zome_manifest.name}} so the Rust module identifier is valid. Ensure
both the use ...:: and cells[...].zome("...") references use the same snake_case
name.
In
`@templates/generic/link-type/dnas/`{{dna_role_name}}/zomes/coordinator/{{coordinator_zome_manifest.name}}/tests/{{`#if`
to_referenceable}}{{snake_case from_referenceable.name}}_to_{{snake_case (plural
to_referenceable.name)}}.rs{{¡if}}.hbs:
- Around line 1-8: The use statement currently imports the coordinator zome with
{{coordinator_zome_manifest.name}} which can produce invalid Rust identifiers if
the zome name contains hyphens; update the template so the crate/module
identifier is snake_case by changing the import to use the snake_cased
coordinator zome name (i.e., replace {{coordinator_zome_manifest.name}} with
{{snake_case coordinator_zome_manifest.name}}) in the line that reads use
{{coordinator_zome_manifest.name}}::{{snake_case
from_referenceable.name}}_to_{{snake_case (plural to_referenceable.name)}}::*;
so the generated Rust crate path is a valid snake_case identifier.
🧹 Nitpick comments (1)
templates/generic/link-type/dnas/{{dna_role_name}}/zomes/coordinator/{{coordinator_zome_manifest.name}}/tests/{{`#if` to_referenceable}}{{snake_case from_referenceable.name}}_to_{{snake_case (plural to_referenceable.name)}}.rs{{¡if}}.hbs (1)
41-44: Remove empty{{else}}block.The empty else block on lines 43-44 serves no purpose and can be removed for cleaner template code.
♻️ Suggested cleanup
{{`#if` (ne to_referenceable.hash_type "ExternalHash")}} let target_record = create_{{snake_case to_referenceable.name}}(&alice_conductor, &alice_zome).await; - {{else}} {{/if}}
...ase from_referenceable.name}}_to_{{snake_case (plural to_referenceable.name)}}.rs{{¡if}}.hbs
Show resolved
Hide resolved
|
✔️ 8c0da1e...dba41e2 - Conventional commits check succeeded. |
ThetaSinner
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Really liking the testing here
Adds sweettest tests when scaffolding link types.
part of #533
Summary by CodeRabbit
Tests
Chores