Fb tests #7
Fb tests #7
Conversation
WalkthroughThis pull request establishes a reference to an external configuration folder by adding a relative path in the core config file. It introduces new test modules to verify the configuration loading functionality. In the login configuration file, tests check both successful loading and proper error handling. Additionally, a new test structure in another module verifies the loading of a separate YAML file. Changes to the Changes
Sequence Diagram(s)sequenceDiagram
participant T as Test
participant LS as LoginServer
participant F as ConfigFile ("../config/login.yaml")
T->>LS: Call load() for valid config
LS->>F: Request YAML configuration
F-->>LS: Return configuration with name "Login server"
LS-->>T: Return config object
sequenceDiagram
participant T as Test
participant LS as LoginServer
participant F as ConfigFile ("./login.yaml")
T->>LS: Call load() for invalid config
LS->>F: Attempt to access YAML file
F-->>LS: File not found error
LS-->>T: Panic with error message
sequenceDiagram
participant T as Test
participant TC as TestConf2
participant F as ConfigFile ("config/game.yaml")
T->>TC: Call load() for Game server config
TC->>F: Request YAML configuration
F-->>TC: Return configuration with name "Game server"
TC-->>T: Return loaded configuration
Poem
✨ Finishing Touches
🪧 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 using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![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
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
l2-core/config(1 hunks)l2-core/src/config/login.rs(1 hunks)l2-core/src/config/traits.rs(2 hunks)
✅ Files skipped from review due to trivial changes (1)
- l2-core/config
🧰 Additional context used
🪛 GitHub Actions: L2Shablya
l2-core/src/config/login.rs
[error] 28-28: Failed to open config file: ./login.yaml. Error: No such file or directory (os error 2).
[error] 114-114: Assertion failed: left == right. Left: "Login server", Right: "login".
🔇 Additional comments (2)
l2-core/src/config/traits.rs (2)
85-86: LGTM!The import hack is a common pattern in Rust for testing macros within the same crate.
95-110:✅ Verification successful
Verify the configuration file path and expected value.
The test implementation looks good. However, ensure that:
- The configuration file exists at
config/game.yamlrelative to the repository root.- The configuration file contains the expected value
"Game server"for thenamefield.
🏁 Script executed:
#!/bin/bash # Description: Verify the configuration file and its content. # Test: Check if the configuration file exists if [ -f "config/game.yaml" ]; then echo "Configuration file exists" # Test: Check if the configuration file contains the expected value if grep -q "name: Game server" "config/game.yaml"; then echo "Configuration file contains the expected value" else echo "Configuration file does not contain the expected value" fi else echo "Configuration file does not exist" fiLength of output: 240
Configuration file and test assertions confirmed.
The configuration file exists at
config/game.yamland contains the expected value"Game server"for thenamefield. The new test struct and function (TestConf2andtest_config_file_load) correctly follow the established pattern.
l2-core/src/config/login.rs
Outdated
| #[cfg(test)] | ||
| mod tests { | ||
| use crate::config::login::LoginServer; | ||
| use crate::traits::ServerConfig; | ||
|
|
||
| #[test] | ||
| fn test_config_load() { | ||
| let cfg = LoginServer::load("../config/login.yaml"); | ||
| assert_eq!(cfg.name, "login"); | ||
| } | ||
| #[test] | ||
| #[should_panic(expected = "Failed to open config file: ./login.yaml. Error: No such file or directory (os error 2). Current directory: /Users/artem/dev/rust/L2Shablya/l2-core")] | ||
| fn test_config_load_err() { | ||
| LoginServer::load("./login.yaml"); | ||
| } | ||
| } |
There was a problem hiding this comment.
Fix test failures.
The test module has two issues:
- The assertion in
test_config_loadfails because the expected value doesn't match the actual value in the configuration file. - The error test
test_config_load_erris brittle due to hardcoded path in the expected error message.
Apply this diff to fix the assertion:
- assert_eq!(cfg.name, "login");
+ assert_eq!(cfg.name, "Login server");And refactor the error test to be more robust:
- #[should_panic(expected = "Failed to open config file: ./login.yaml. Error: No such file or directory (os error 2). Current directory: /Users/artem/dev/rust/L2Shablya/l2-core")]
+ #[should_panic(expected = "Failed to open config file: ./login.yaml")]
fn test_config_load_err() {
LoginServer::load("./login.yaml");
}📝 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.
| #[cfg(test)] | |
| mod tests { | |
| use crate::config::login::LoginServer; | |
| use crate::traits::ServerConfig; | |
| #[test] | |
| fn test_config_load() { | |
| let cfg = LoginServer::load("../config/login.yaml"); | |
| assert_eq!(cfg.name, "login"); | |
| } | |
| #[test] | |
| #[should_panic(expected = "Failed to open config file: ./login.yaml. Error: No such file or directory (os error 2). Current directory: /Users/artem/dev/rust/L2Shablya/l2-core")] | |
| fn test_config_load_err() { | |
| LoginServer::load("./login.yaml"); | |
| } | |
| } | |
| #[cfg(test)] | |
| mod tests { | |
| use crate::config::login::LoginServer; | |
| use crate::traits::ServerConfig; | |
| #[test] | |
| fn test_config_load() { | |
| let cfg = LoginServer::load("../config/login.yaml"); | |
| assert_eq!(cfg.name, "Login server"); | |
| } | |
| #[test] | |
| #[should_panic(expected = "Failed to open config file: ./login.yaml")] | |
| fn test_config_load_err() { | |
| LoginServer::load("./login.yaml"); | |
| } | |
| } |
🧰 Tools
🪛 GitHub Actions: L2Shablya
[error] 114-114: Assertion failed: left == right. Left: "Login server", Right: "login".
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
l2-core/src/traits/server.rs (1)
167-348: Well-structured test module with comprehensive mocking.The test module provides good coverage of the
Servertrait functionality with appropriate mocks and test cases.Consider implementing critical mock methods.
While
todo!()is acceptable for most mock methods, consider implementing critical methods that might be needed for future tests:
handle_client()on_connect()on_disconnect()async fn handle_client(&mut self) -> anyhow::Result<()> { - todo!() + Ok(()) } async fn on_connect(&mut self) -> anyhow::Result<()> { - todo!() + Ok(()) } async fn on_disconnect(&mut self) { - todo!() + // No-op implementation }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (11)
Cargo.toml(1 hunks)entities/Cargo.toml(1 hunks)game/Cargo.toml(1 hunks)game/src/packets/from_client/char_create.rs(1 hunks)l2-core/Cargo.toml(1 hunks)l2-core/src/dto.rs(2 hunks)l2-core/src/traits/server.rs(1 hunks)login/Cargo.toml(1 hunks)macro-common/Cargo.toml(1 hunks)migration/Cargo.toml(1 hunks)test-utils/Cargo.toml(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- game/src/packets/from_client/char_create.rs
🔇 Additional comments (10)
l2-core/src/dto.rs (1)
7-7: LGTM! Good addition ofDefaulttrait.Adding the
Defaulttrait toDatabaseandRuntimestructs enhances their usability in testing scenarios by allowing partial initialization of these configuration structures.Also applies to: 33-33
test-utils/Cargo.toml (1)
4-4: LGTM! Good workspace configuration update.Moving to workspace-level edition configuration improves consistency across the project.
macro-common/Cargo.toml (1)
4-4: LGTM! Good workspace configuration update.Moving to workspace-level edition configuration improves consistency across the project.
entities/Cargo.toml (1)
4-4: Review the Workspace Edition Directive
The replacement of the traditional edition field withedition.workspace = truesignals an intention to inherit the Rust edition from the workspace configuration. Please verify that your Cargo tooling and workspace settings support this new directive and that it behaves as expected across your workspace.migration/Cargo.toml (1)
4-4: Confirm Workspace Edition Configuration in Migration Package
The modified line usingedition.workspace = truein the[package]section should ensure that the migration package adheres to the workspace’s edition settings. Double-check that any workspace-level configurations are correctly applied and that this syntax is supported by your Cargo version.login/Cargo.toml (1)
4-4: Verify Workspace Edition Setting for Login Package
Switching toedition.workspace = truealigns this package with the workspace’s configuration. Please confirm that this change integrates correctly with the overall workspace settings and that there are no unintended side effects with dependency resolution or compilation.game/Cargo.toml (1)
4-4: Review Workspace Edition Usage in Game Package
The change toedition.workspace = trueis consistent with the new workspace-centric approach. Ensure that the game package now correctly inherits the Rust edition from the workspace and that your build environment supports this configuration.l2-core/Cargo.toml (1)
4-4: Assess the Workspace Edition Directive in l2-core
The alteration toedition.workspace = trueis intended to unify the edition setting across the workspace. Verify that this directive is compatible with your Cargo setup and that all team members are aware of the change in configuration management for editions.Cargo.toml (2)
1-3: New Workspace Package Configuration AddedThe addition of a
[workspace.package]section withedition = "2021"provides a unified Rust edition configuration for the workspace. This change helps ensure consistency across all workspace packages. Please verify that your local Cargo version supports this configuration for smooth builds.
8-8: Improved Formatting with a NewlineAn empty newline was added at line 8 to separate sections and enhance readability. This minor formatting update helps visually organize the file.
Summary by CodeRabbit
These updates enhance the system's reliability and ensure that configuration files are loaded and validated correctly.