8000 feat: support deps for .mbt.md by Young-Flash · Pull Request #832 · moonbitlang/moon · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

feat: support deps for .mbt.md #832

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

Merged
merged 3 commits into from
May 28, 2025
Merged

feat: support deps for .mbt.md #832

merged 3 commits into from
May 28, 2025

Conversation

Young-Flash
Copy link
Collaborator

No description provided.

Copy link
peter-jerry-ye-code-review bot commented May 28, 2025
Function has too many parameters and responsibilities

Category
Maintainability
Code Snippet
fn run_test_in_single_file(cli: &UniversalFlags, cmd: &TestSubcommand) -> anyhow::Result
Recommendation
Split the function into smaller focused functions handling either configuration parsing, test setup, or test execution
Reasoning
The function is ~100 lines long and handles multiple concerns including path resolution, config parsing, test setup and execution. This makes it harder to maintain and test individual components.

Potential race condition in file operations

Category
Correctness
Code Snippet
let path = "1.txt"
@fs.write_string_to_file!(path, content)
let res = @fs.read_file_to_string!(path)
Recommendation
Add file locking mechanism or use a unique temporary file path for each test run
Reasoning
Multiple concurrent test runs could try to write/read from the same file path, leading to race conditions and flaky tests

Unnecessary cloning in hot path

Category
Performance
Code Snippet
deps.insert(k.to_string(), v.clone().into());
Recommendation
Use references where possible and only clone when necessary:
deps.insert(k.as_str().to_owned(), (*v).into());
Reasoning
The current implementation clones both the key and value unnecessarily in a loop when constructing dependencies map

@Young-Flash Young-Flash force-pushed the single_mbt_md_deps branch 2 times, most recently from 6056a7a to c38f213 Compare May 28, 2025 06:31
@Young-Flash Young-Flash force-pushed the single_mbt_md_deps branch from c38f213 to 924d710 Compare May 28, 2025 06:37
@Young-Flash Young-Flash enabled auto-merge May 28, 2025 07:28
@Young-Flash Young-Flash merged commit e6f084e into main May 28, 2025
23 of 25 checks passed
@Young-Flash Young-Flash deleted the single_mbt_md_deps branch May 28, 2025 07:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant
0