hakowork/rojo
3
0
Fork 0
mirror of https://github.com/rojo-rbx/rojo.git synced 2026-10-23 05:51:39 +00:00
Code Issues Projects Releases Packages Wiki Activity Actions

Enhance logging and fix regression in TODO.txt

Browse source 
- Updated the logging format in TODO.txt to provide clearer information about file operations and potential issues, including warnings about lossy string conversions.
- Added a note to address a regression in logging behavior that may affect users relying on specific property formats.
- Refactored the `flatten_tree` function in `git.rs` to improve parameter clarity by renaming the repository argument.
- Updated the `mcp.rs` test to utilize a more concise method for checking integer types in the timeout configuration.
- Introduced a new method in `TestServeSession` to fetch git metadata, improving the test suite's ability to validate git-related functionality.

These changes aim to improve logging clarity, address regressions, and enhance test coverage for git metadata handling.
This commit is contained in:
Joe 2026-03-01 17:02:57 -06:00
parent 059abd1245
commit f24e991aca
5 changed files with 64 additions and 43 deletions
Download patch file Download diff file Expand all files Collapse all files

15
TODO.txt
View file

@ -310,6 +310,21 @@ Worst case - we reconcile only on connect, but do so in mega-parallel
Also, we need to explore how VSCode deals with this. They seem to have ZERO issues. Also, we need to explore how VSCode deals with this. They seem to have ZERO issues.
Maybe we can have a mode to tie into the vscode file watcher ourselves? luau-lsp does this. Maybe we can have a mode to tie into the vscode file watcher ourselves? luau-lsp does this.
[logging]
console logging is now:
INFO Input file 'd:\UserGenerated\steal-a-brainrot\.\Project.rbxl' not found, downloading place 120148879522453...
INFO Loaded cookie from Windows Credentials (user_id: 2882755487).
INFO Downloaded in 0.77s
WARN Performing lossy string conversion on property StringValue.Value because it did not contain UTF-8.
This may cause unexpected or broken behavior in your final results if you rely on this property being non UTF-8.
when it used to be this format:
[INFO librojo::change_processor] compute_and_apply_changes: checking path d:\UserGenerated\escape-tsunami\src\ReplicatedStorage\Client\Controllers\UI\CurrenciesUI.luau for instance Ref(Some(162268450181648175032536457080084761373))
[INFO librojo::change_processor] compute_and_apply_changes: path EXISTS via VFS, re-snapshotting d:\UserGenerated\escape-tsunami\src\ReplicatedStorage\Client\Controllers\UI\CurrenciesUI.luau
[INFO librojo::change_processor] VFS event applied: no changes
[INFO librojo::change_processor] Tree reconciliation: corrected 4 added, 4 removed (12.3s)
please fix this regression.
[fix dedup] [fix dedup]
dedup is broken due to relying on sort order dedup is broken due to relying on sort order
recreate broken state by forcing random order in all matching dedup systems of the incoming children list recreate broken state by forcing random order in all matching dedup systems of the incoming children list

2
src/git.rs
View file

@ -212,7 +212,7 @@ fn collect_tree_index_changes(
} }
fn flatten_tree( fn flatten_tree(
repo: &gix::Repository, _repo: &gix::Repository,
tree: &gix::Tree<'_>, tree: &gix::Tree<'_>,
prefix: &str, prefix: &str,
out: &mut HashMap<String, gix::ObjectId>, out: &mut HashMap<String, gix::ObjectId>,

2
src/web/mcp.rs
View file

@ -2393,7 +2393,7 @@ mod tests {
let has_integer = timeout["type"] == "integer" let has_integer = timeout["type"] == "integer"
|| timeout["type"] || timeout["type"]
.as_array() .as_array()
.map_or(false, |arr| arr.iter().any(|v| v == "integer")); .is_some_and(|arr| arr.iter().any(|v| v == "integer"));
assert!(has_integer, "timeout should accept integer: {timeout}"); assert!(has_integer, "timeout should accept integer: {timeout}");
} }
} }

11
tests/rojo_test/serve_util.rs
View file

@ -16,8 +16,8 @@ use serde::Deserialize;
use tempfile::{tempdir, TempDir}; use tempfile::{tempdir, TempDir};
use librojo::web_api::{ use librojo::web_api::{
ReadResponse, SerializeResponse, ServerInfoResponse, SocketPacket, SocketPacketBody, GitMetadata, ReadResponse, SerializeResponse, ServerInfoResponse, SocketPacket,
SocketPacketType, SocketPacketBody, SocketPacketType,
}; };
use rojo_insta_ext::RedactionMap; use rojo_insta_ext::RedactionMap;
@ -340,6 +340,13 @@ impl TestServeSession {
Ok(deserialize_msgpack(&body).expect("Server returned malformed response")) Ok(deserialize_msgpack(&body).expect("Server returned malformed response"))
} }
pub fn get_api_git_metadata(&self) -> Result<Option<GitMetadata>, reqwest::Error> {
let url = format!("http://localhost:{}/api/git-metadata", self.port);
let body = reqwest::blocking::get(url)?.bytes()?;
Ok(deserialize_msgpack(&body).expect("Server returned malformed response"))
}
pub fn get_api_read(&self, id: Ref) -> Result<ReadResponse<'_>, reqwest::Error> { pub fn get_api_read(&self, id: Ref) -> Result<ReadResponse<'_>, reqwest::Error> {
let url = format!("http://localhost:{}/api/read/{}", self.port, id); let url = format!("http://localhost:{}/api/read/{}", self.port, id);
let body = reqwest::blocking::get(url)?.bytes()?; let body = reqwest::blocking::get(url)?.bytes()?;

77
tests/tests/git_sync_defaults.rs
View file

@ -1,6 +1,6 @@
//! Integration tests for git-based sync direction defaults. //! Integration tests for git-based sync direction defaults.
//! //!
//! Tests the full pipeline: git metadata computation, ServerInfoResponse, //! Tests the full pipeline: git metadata computation via /api/git-metadata,
//! stage_ids in WriteRequest, and auto-staging behavior. //! stage_ids in WriteRequest, and auto-staging behavior.
//! //!
//! These tests create real git repos in temp directories to verify: //! These tests create real git repos in temp directories to verify:
@ -13,7 +13,6 @@
use std::collections::HashMap; use std::collections::HashMap;
use std::path::{Path, PathBuf}; use std::path::{Path, PathBuf};
use std::process::Command;
use std::time::Duration; use std::time::Duration;
use std::{fs, thread}; use std::{fs, thread};
@ -70,9 +69,10 @@ fn git_metadata_present_when_in_git_repo() {
let mut session = TestServeSession::new_with_git("git_sync_defaults", |path| { let mut session = TestServeSession::new_with_git("git_sync_defaults", |path| {
git_commit_all(path, "initial commit"); git_commit_all(path, "initial commit");
}); });
let info = session.wait_to_come_online(); let _info = session.wait_to_come_online();
let git_meta = session.get_api_git_metadata().unwrap();
assert!( assert!(
info.git_metadata.is_some(), git_meta.is_some(),
"gitMetadata should be present when project is in a git repo" "gitMetadata should be present when project is in a git repo"
); );
} }
@ -80,9 +80,10 @@ fn git_metadata_present_when_in_git_repo() {
#[test] #[test]
fn git_metadata_absent_when_not_in_git_repo() { fn git_metadata_absent_when_not_in_git_repo() {
let mut session = TestServeSession::new("git_sync_defaults"); let mut session = TestServeSession::new("git_sync_defaults");
let info = session.wait_to_come_online(); let _info = session.wait_to_come_online();
let git_meta = session.get_api_git_metadata().unwrap();
assert!( assert!(
info.git_metadata.is_none(), git_meta.is_none(),
"gitMetadata should be None when project is not in a git repo" "gitMetadata should be None when project is not in a git repo"
); );
} }
@ -97,8 +98,8 @@ fn no_changes_yields_empty_changed_ids() {
git_commit_all(path, "initial commit"); git_commit_all(path, "initial commit");
// No modifications after commit // No modifications after commit
}); });
let info = session.wait_to_come_online(); let _info = session.wait_to_come_online();
let meta = info.git_metadata.unwrap(); let meta = session.get_api_git_metadata().unwrap().unwrap();
assert!( assert!(
meta.changed_ids.is_empty(), meta.changed_ids.is_empty(),
"changedIds should be empty when no files changed" "changedIds should be empty when no files changed"
@ -112,8 +113,8 @@ fn unstaged_script_modification_appears_in_changed_ids() {
git_commit_all(path, "initial commit"); git_commit_all(path, "initial commit");
fs::write(path.join("src/ModuleA.luau"), "-- modified\nreturn {}").unwrap(); fs::write(path.join("src/ModuleA.luau"), "-- modified\nreturn {}").unwrap();
}); });
let info = session.wait_to_come_online(); let _info = session.wait_to_come_online();
let meta = info.git_metadata.unwrap(); let meta = session.get_api_git_metadata().unwrap().unwrap();
assert!( assert!(
!meta.changed_ids.is_empty(), !meta.changed_ids.is_empty(),
@ -128,8 +129,8 @@ fn staged_script_modification_appears_in_changed_ids() {
fs::write(path.join("src/ModuleA.luau"), "-- staged mod\nreturn {}").unwrap(); fs::write(path.join("src/ModuleA.luau"), "-- staged mod\nreturn {}").unwrap();
git_stage(path, "src/ModuleA.luau"); git_stage(path, "src/ModuleA.luau");
}); });
let info = session.wait_to_come_online(); let _info = session.wait_to_come_online();
let meta = info.git_metadata.unwrap(); let meta = session.get_api_git_metadata().unwrap().unwrap();
assert!(!meta.changed_ids.is_empty()); assert!(!meta.changed_ids.is_empty());
} }
@ -140,8 +141,8 @@ fn untracked_file_appears_in_changed_ids() {
git_commit_all(path, "initial commit"); git_commit_all(path, "initial commit");
fs::write(path.join("src/NewScript.luau"), "return 'new'").unwrap(); fs::write(path.join("src/NewScript.luau"), "return 'new'").unwrap();
}); });
let info = session.wait_to_come_online(); let _info = session.wait_to_come_online();
let meta = info.git_metadata.unwrap(); let meta = session.get_api_git_metadata().unwrap().unwrap();
assert!( assert!(
!meta.changed_ids.is_empty(), !meta.changed_ids.is_empty(),
@ -157,7 +158,7 @@ fn unchanged_files_not_in_changed_ids() {
fs::write(path.join("src/ModuleA.luau"), "-- changed").unwrap(); fs::write(path.join("src/ModuleA.luau"), "-- changed").unwrap();
}); });
let info = session.wait_to_come_online(); let info = session.wait_to_come_online();
let meta = info.git_metadata.unwrap(); let meta = session.get_api_git_metadata().unwrap().unwrap();
let read = session.get_api_read(info.root_instance_id).unwrap(); let read = session.get_api_read(info.root_instance_id).unwrap();
@ -182,7 +183,7 @@ fn non_script_change_in_changed_ids_but_no_hash() {
.unwrap(); .unwrap();
}); });
let info = session.wait_to_come_online(); let info = session.wait_to_come_online();
let meta = info.git_metadata.unwrap(); let meta = session.get_api_git_metadata().unwrap().unwrap();
let read = session.get_api_read(info.root_instance_id).unwrap(); let read = session.get_api_read(info.root_instance_id).unwrap();
if let Some((model_id, _)) = find_instance_by_name(&read.instances, "SimpleModel") { if let Some((model_id, _)) = find_instance_by_name(&read.instances, "SimpleModel") {
@ -211,7 +212,7 @@ fn committed_hash_matches_head_content() {
fs::write(path.join("src/ModuleA.luau"), "-- v2\nreturn {}").unwrap(); fs::write(path.join("src/ModuleA.luau"), "-- v2\nreturn {}").unwrap();
}); });
let info = session.wait_to_come_online(); let info = session.wait_to_come_online();
let meta = info.git_metadata.unwrap(); let meta = session.get_api_git_metadata().unwrap().unwrap();
let read = session.get_api_read(info.root_instance_id).unwrap(); let read = session.get_api_read(info.root_instance_id).unwrap();
if let Some((module_id, _)) = find_instance_by_name(&read.instances, "ModuleA") { if let Some((module_id, _)) = find_instance_by_name(&read.instances, "ModuleA") {
@ -240,7 +241,7 @@ fn staged_hash_included_when_different_from_head() {
fs::write(path.join("src/ModuleA.luau"), "-- working tree version").unwrap(); fs::write(path.join("src/ModuleA.luau"), "-- working tree version").unwrap();
}); });
let info = session.wait_to_come_online(); let info = session.wait_to_come_online();
let meta = info.git_metadata.unwrap(); let meta = session.get_api_git_metadata().unwrap().unwrap();
let read = session.get_api_read(info.root_instance_id).unwrap(); let read = session.get_api_read(info.root_instance_id).unwrap();
if let Some((module_id, _)) = find_instance_by_name(&read.instances, "ModuleA") { if let Some((module_id, _)) = find_instance_by_name(&read.instances, "ModuleA") {
@ -278,7 +279,7 @@ fn staged_hash_deduplicated_when_same_as_head() {
// Reset the stage to match HEAD content // Reset the stage to match HEAD content
}); });
let info = session.wait_to_come_online(); let info = session.wait_to_come_online();
let meta = info.git_metadata.unwrap(); let meta = session.get_api_git_metadata().unwrap().unwrap();
let read = session.get_api_read(info.root_instance_id).unwrap(); let read = session.get_api_read(info.root_instance_id).unwrap();
if let Some((module_id, _)) = find_instance_by_name(&read.instances, "ModuleA") { if let Some((module_id, _)) = find_instance_by_name(&read.instances, "ModuleA") {
@ -297,7 +298,7 @@ fn new_untracked_script_has_no_committed_hash() {
fs::write(path.join("src/BrandNew.luau"), "return 'brand new'").unwrap(); fs::write(path.join("src/BrandNew.luau"), "return 'brand new'").unwrap();
}); });
let info = session.wait_to_come_online(); let info = session.wait_to_come_online();
let meta = info.git_metadata.unwrap(); let meta = session.get_api_git_metadata().unwrap().unwrap();
let read = session.get_api_read(info.root_instance_id).unwrap(); let read = session.get_api_read(info.root_instance_id).unwrap();
if let Some((new_id, _)) = find_instance_by_name(&read.instances, "BrandNew") { if let Some((new_id, _)) = find_instance_by_name(&read.instances, "BrandNew") {
@ -322,8 +323,8 @@ fn init_style_script_appears_in_changed_ids() {
git_commit_all(path, "initial commit"); git_commit_all(path, "initial commit");
fs::write(path.join("src/DirModule/init.luau"), "-- modified init").unwrap(); fs::write(path.join("src/DirModule/init.luau"), "-- modified init").unwrap();
}); });
let info = session.wait_to_come_online(); let _info = session.wait_to_come_online();
let meta = info.git_metadata.unwrap(); let meta = session.get_api_git_metadata().unwrap().unwrap();
assert!( assert!(
!meta.changed_ids.is_empty(), !meta.changed_ids.is_empty(),
@ -340,7 +341,7 @@ fn init_style_script_has_committed_hash() {
fs::write(path.join("src/DirModule/init.luau"), "-- v2").unwrap(); fs::write(path.join("src/DirModule/init.luau"), "-- v2").unwrap();
}); });
let info = session.wait_to_come_online(); let info = session.wait_to_come_online();
let meta = info.git_metadata.unwrap(); let meta = session.get_api_git_metadata().unwrap().unwrap();
let read = session.get_api_read(info.root_instance_id).unwrap(); let read = session.get_api_read(info.root_instance_id).unwrap();
if let Some((dir_id, _)) = find_instance_by_name(&read.instances, "DirModule") { if let Some((dir_id, _)) = find_instance_by_name(&read.instances, "DirModule") {
@ -373,7 +374,7 @@ fn all_script_types_get_hashes() {
fs::write(path.join("src/ClientScript.client.luau"), "-- mod").unwrap(); fs::write(path.join("src/ClientScript.client.luau"), "-- mod").unwrap();
}); });
let info = session.wait_to_come_online(); let info = session.wait_to_come_online();
let meta = info.git_metadata.unwrap(); let meta = session.get_api_git_metadata().unwrap().unwrap();
let read = session.get_api_read(info.root_instance_id).unwrap(); let read = session.get_api_read(info.root_instance_id).unwrap();
for name in &["ModuleA", "ServerScript", "ClientScript"] { for name in &["ModuleA", "ServerScript", "ClientScript"] {
@ -593,8 +594,8 @@ fn reconnect_gets_fresh_git_metadata() {
let mut session = TestServeSession::new_with_git("git_sync_defaults", |path| { let mut session = TestServeSession::new_with_git("git_sync_defaults", |path| {
git_commit_all(path, "initial commit"); git_commit_all(path, "initial commit");
}); });
let info1 = session.wait_to_come_online(); let _info = session.wait_to_come_online();
let meta1 = info1.git_metadata.unwrap(); let meta1 = session.get_api_git_metadata().unwrap().unwrap();
assert!(meta1.changed_ids.is_empty(), "No changes initially"); assert!(meta1.changed_ids.is_empty(), "No changes initially");
// Now modify a file on disk // Now modify a file on disk
@ -604,9 +605,8 @@ fn reconnect_gets_fresh_git_metadata() {
) )
.unwrap(); .unwrap();
// Re-query the API (simulates reconnect) // Re-query the git metadata (simulates reconnect)
let info2 = session.get_api_rojo().unwrap(); let meta2 = session.get_api_git_metadata().unwrap().unwrap();
let meta2 = info2.git_metadata.unwrap();
assert!( assert!(
!meta2.changed_ids.is_empty(), !meta2.changed_ids.is_empty(),
"Fresh connect should reflect new git changes" "Fresh connect should reflect new git changes"
@ -622,11 +622,11 @@ fn empty_repo_no_commits_returns_metadata() {
let mut session = TestServeSession::new_with_git("git_sync_defaults", |_path| { let mut session = TestServeSession::new_with_git("git_sync_defaults", |_path| {
// Don't commit anything -- repo has no HEAD // Don't commit anything -- repo has no HEAD
}); });
let info = session.wait_to_come_online(); let _info = session.wait_to_come_online();
// Should still get metadata (untracked files) // Should still get metadata (untracked files)
// or None if git_changed_files returns None for no-HEAD repos // or None if git_changed_files returns None for no-HEAD repos
// Either way, should not panic // Either way, should not panic
let _ = info.git_metadata; let _ = session.get_api_git_metadata();
} }
#[test] #[test]
@ -637,8 +637,8 @@ fn multiple_files_changed_all_tracked() {
fs::write(path.join("src/ServerScript.server.luau"), "-- mod server").unwrap(); fs::write(path.join("src/ServerScript.server.luau"), "-- mod server").unwrap();
fs::write(path.join("src/ClientScript.client.luau"), "-- mod client").unwrap(); fs::write(path.join("src/ClientScript.client.luau"), "-- mod client").unwrap();
}); });
let info = session.wait_to_come_online(); let _info = session.wait_to_come_online();
let meta = info.git_metadata.unwrap(); let meta = session.get_api_git_metadata().unwrap().unwrap();
assert!( assert!(
meta.changed_ids.len() >= 3, meta.changed_ids.len() >= 3,
@ -963,8 +963,8 @@ fn committed_changes_still_in_changed_ids_via_initial_head() {
let mut session = TestServeSession::new_with_git("git_sync_defaults", |path| { let mut session = TestServeSession::new_with_git("git_sync_defaults", |path| {
git_commit_all(path, "initial commit"); git_commit_all(path, "initial commit");
}); });
let info1 = session.wait_to_come_online(); let _info = session.wait_to_come_online();
let meta1 = info1.git_metadata.unwrap(); let meta1 = session.get_api_git_metadata().unwrap().unwrap();
assert!(meta1.changed_ids.is_empty(), "No changes initially"); assert!(meta1.changed_ids.is_empty(), "No changes initially");
fs::write( fs::write(
@ -974,8 +974,7 @@ fn committed_changes_still_in_changed_ids_via_initial_head() {
.unwrap(); .unwrap();
git_commit_all(session.path(), "edit after serve"); git_commit_all(session.path(), "edit after serve");
let info2 = session.get_api_rojo().unwrap(); let meta2 = session.get_api_git_metadata().unwrap().unwrap();
let meta2 = info2.git_metadata.unwrap();
assert!( assert!(
!meta2.changed_ids.is_empty(), !meta2.changed_ids.is_empty(),
"Committed changes should still appear in changedIds via initial_head diff" "Committed changes should still appear in changedIds via initial_head diff"
@ -995,8 +994,8 @@ fn committed_script_gets_initial_head_hash() {
fs::write(session.path().join("src/ModuleA.luau"), modified_content).unwrap(); fs::write(session.path().join("src/ModuleA.luau"), modified_content).unwrap();
git_commit_all(session.path(), "edit after serve"); git_commit_all(session.path(), "edit after serve");
let meta2 = session.get_api_git_metadata().unwrap().unwrap();
let info2 = session.get_api_rojo().unwrap(); let info2 = session.get_api_rojo().unwrap();
let meta2 = info2.git_metadata.unwrap();
let read = session.get_api_read(info2.root_instance_id).unwrap(); let read = session.get_api_read(info2.root_instance_id).unwrap();
if let Some((module_id, _)) = find_instance_by_name(&read.instances, "ModuleA") { if let Some((module_id, _)) = find_instance_by_name(&read.instances, "ModuleA") {
@ -1039,8 +1038,8 @@ fn multiple_commits_after_session_start_all_tracked() {
.unwrap(); .unwrap();
git_commit_all(session.path(), "commit 2"); git_commit_all(session.path(), "commit 2");
let meta2 = session.get_api_git_metadata().unwrap().unwrap();
let info2 = session.get_api_rojo().unwrap(); let info2 = session.get_api_rojo().unwrap();
let meta2 = info2.git_metadata.unwrap();
let read = session.get_api_read(info2.root_instance_id).unwrap(); let read = session.get_api_read(info2.root_instance_id).unwrap();
if let Some((module_id, _)) = find_instance_by_name(&read.instances, "ModuleA") { if let Some((module_id, _)) = find_instance_by_name(&read.instances, "ModuleA") {