feat: add macros for testing #2337
Conversation
WalkthroughOhayo, sensei! The recent changes involve the introduction of two new inline macro plugins, Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Plugin
participant Utils
participant Config
User->>Plugin: Invoke get_models_test_class_hashes!
Plugin->>Utils: Load manifest models and namespaces
Utils-->>Plugin: Return models and namespaces
Plugin-->>User: Return class hashes
User->>Plugin: Invoke spawn_test_world_full!
Plugin->>Utils: Load models and namespaces
Utils-->>Plugin: Return models and namespaces
Plugin-->>User: Spawn test world
Recent review detailsConfiguration used: .coderabbit.yaml Files ignored due to path filters (2)
Files selected for processing (12)
Files skipped from review due to trivial changes (1)
Additional comments not posted (14)
TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add Documentation and Community
|
|
Wondering if two macros is necessary... // Registers everything.
let world = spawn_test_world!();
// Registers only the given namespace and it's models.
let world = spawn_test_world!(["dojo_examples"]);But not sure if having access to |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
Outside diff range, codebase verification and nitpick comments (2)
crates/dojo-lang/src/inline_macros/spawn_test_world_full.rs (1)
62-84: Ohayo, sensei! Review code generation logic.The code generation logic for spawning the test world is well-implemented. Consider adding comments to explain the purpose of each step for future maintainability.
crates/dojo-lang/src/inline_macros/get_models_test_class_hashes.rs (1)
1-139: Great implementation with robust error handling!The
GetModelsTestClassHashesmacro plugin is well-implemented with efficient use ofPatchBuilderfor code generation. Error handling and diagnostics are thorough.Consider improving error messages for clarity.
Some error messages could be more descriptive to aid debugging. For example, specifying which argument caused the error could be helpful.
| /// Reads all the models and namespaces from base manifests files. | ||
| pub fn load_manifest_models_and_namespaces( | ||
| cfg_set: &CfgSet, | ||
| whitelisted_namespaces: &[String], | ||
| ) -> anyhow::Result<(Vec<String>, Vec<String>)> { | ||
| let dojo_manifests_dir = get_dojo_manifests_dir(cfg_set.clone())?; | ||
|
|
||
| let base_dir = dojo_manifests_dir.join("base"); | ||
| let base_abstract_manifest = BaseManifest::load_from_path(&base_dir)?; | ||
|
|
||
| let mut models = HashSet::new(); | ||
| let mut namespaces = HashSet::new(); | ||
|
|
||
| for model in base_abstract_manifest.models { | ||
| let qualified_path = model.inner.qualified_path; | ||
| let namespace = naming::split_tag(&model.inner.tag)?.0; | ||
|
|
||
| if !whitelisted_namespaces.is_empty() && !whitelisted_namespaces.contains(&namespace) { | ||
| continue; | ||
| } | ||
|
|
||
| models.insert(qualified_path); | ||
| namespaces.insert(namespace); | ||
| } | ||
|
|
||
| let models_vec: Vec<String> = models.into_iter().collect(); | ||
| let namespaces_vec: Vec<String> = namespaces.into_iter().collect(); | ||
|
|
||
| Ok((namespaces_vec, models_vec)) | ||
| } |
There was a problem hiding this comment.
Ohayo, sensei! Improve error messaging.
The load_manifest_models_and_namespaces function handles manifest loading and filtering. Consider enhancing error messages to provide more context on failures, such as which file or namespace caused the issue.
| /// Gets the dojo_manifests_dir from the cfg_set. | ||
| pub fn get_dojo_manifests_dir(cfg_set: CfgSet) -> anyhow::Result<Utf8PathBuf> { | ||
| for cfg in cfg_set.into_iter() { | ||
| if cfg.key == DOJO_MANIFESTS_DIR_CFG_KEY { | ||
| return Ok(Utf8PathBuf::from(cfg.value.unwrap().as_str().to_string())); | ||
| } | ||
| } | ||
|
|
||
| Err(anyhow::anyhow!("dojo_manifests_dir not found")) | ||
| } |
There was a problem hiding this comment.
Ohayo, sensei! Ensure robust error handling.
The get_dojo_manifests_dir function retrieves the directory path. Ensure that the error message when the key is not found is informative enough for debugging purposes.
| let (namespaces, models) = match load_manifest_models_and_namespaces(metadata.cfg_set, &[]) | ||
| { | ||
| Ok((namespaces, models)) => (namespaces, models), | ||
| Err(_e) => { | ||
| return InlinePluginResult { | ||
| code: None, | ||
| diagnostics: vec![PluginDiagnostic { | ||
| stable_ptr: syntax.stable_ptr().untyped(), | ||
| message: "Failed to load models and namespaces, ensure you have run `sozo \ | ||
| build` first." | ||
| .to_string(), | ||
| severity: Severity::Error, | ||
| }], | ||
| }; | ||
| } | ||
| }; |
There was a problem hiding this comment.
Ohayo, sensei! Enhance error feedback.
When loading models and namespaces fails, consider providing more detailed error feedback, possibly including the specific reason for the failure.
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #2337 +/- ##
==========================================
+ Coverage 66.13% 67.19% +1.05%
==========================================
Files 354 357 +3
Lines 46585 46622 +37
==========================================
+ Hits 30808 31326 +518
+ Misses 15777 15296 -481 ☔ View full report in Codecov by Sentry. |
This PR adds some useful macro to speed up testing setup.
A painful point in testing setup is spawning the test world with all the namespaces and models registered. This can quickly become hard to track when the number of models increases.
An important consideration to use the two macros below is that,
sozo buildmust be run first. The compiler will remind it to the user if the project is not built first.get_models_test_class_hashesThis macro gives the possibility to get the
TEST_CLASS_HASHfor all the models or only for the given namespaces.spawn_test_worldThis macro has only namespaces as control, and registers all the namespaces and models inside the given namespaces (or everything). As the dojo world deployment and registration of models can take a long time, this macro also allow filtering by namespaces.
Summary by CodeRabbit
New Features
GetModelsTestClassHashesandSpawnTestWorldFull, enhancing macro capabilities in the Cairo programming language.Bug Fixes
Documentation