settings_ui: Add test for default values (#37466)

Ben Kunkle created

Closes #ISSUE

Adds a test that checks that all settings have default values in
`default.json`. Currently only tests that settings supported by
SettingsUi have defaults, as more settings are added to the settings
editor they will be added to the test as well.

Release Notes:

- N/A *or* Added/Fixed/Improved ...

Change summary

Cargo.lock                              |  1 
crates/settings/src/settings_ui_core.rs |  3 
crates/settings_ui/Cargo.toml           |  5 +
crates/settings_ui/src/settings_ui.rs   | 68 +++++++++++++++++++++++++-
crates/zed/Cargo.toml                   |  1 
crates/zed/src/zed.rs                   | 30 +++++++++++
6 files changed, 102 insertions(+), 6 deletions(-)

Detailed changes

Cargo.lock 🔗

@@ -14906,6 +14906,7 @@ version = "0.1.0"
 dependencies = [
  "anyhow",
  "command_palette_hooks",
+ "debugger_ui",
  "editor",
  "feature_flags",
  "gpui",

crates/settings/src/settings_ui_core.rs 🔗

@@ -27,6 +27,7 @@ pub struct SettingsUiEntry {
     /// The path in the settings JSON file for this setting. Relative to parent
     /// None implies `#[serde(flatten)]` or `Settings::KEY.is_none()` for top level settings
     pub path: Option<&'static str>,
+    /// What is displayed for the text for this entry
     pub title: &'static str,
     pub item: SettingsUiItem,
 }
@@ -95,7 +96,7 @@ impl<T: serde::Serialize> SettingsValue<T> {
 
 pub struct SettingsUiItemDynamic {
     pub options: Vec<SettingsUiEntry>,
-    pub determine_option: fn(&serde_json::Value, &mut App) -> usize,
+    pub determine_option: fn(&serde_json::Value, &App) -> usize,
 }
 
 pub struct SettingsUiItemGroup {

crates/settings_ui/Cargo.toml 🔗

@@ -13,6 +13,7 @@ path = "src/settings_ui.rs"
 
 [features]
 default = []
+test-support = []
 
 [dependencies]
 anyhow.workspace = true
@@ -29,6 +30,10 @@ ui.workspace = true
 workspace.workspace = true
 workspace-hack.workspace = true
 
+
+[dev-dependencies]
+debugger_ui.workspace = true
+
 # Uncomment other workspace dependencies as needed
 # assistant.workspace = true
 # client.workspace = true

crates/settings_ui/src/settings_ui.rs 🔗

@@ -151,7 +151,9 @@ struct UiEntry {
     next_sibling: Option<usize>,
     // expanded: bool,
     render: Option<SettingsUiItemSingle>,
-    select_descendant: Option<fn(&serde_json::Value, &mut App) -> usize>,
+    /// For dynamic items this is a way to select a value from a list of values
+    /// this is always none for non-dynamic items
+    select_descendant: Option<fn(&serde_json::Value, &App) -> usize>,
 }
 
 impl UiEntry {
@@ -177,7 +179,7 @@ impl UiEntry {
     }
 }
 
-struct SettingsUiTree {
+pub struct SettingsUiTree {
     root_entry_indices: Vec<usize>,
     entries: Vec<UiEntry>,
     active_entry_index: usize,
@@ -242,7 +244,7 @@ fn build_tree_item(
 }
 
 impl SettingsUiTree {
-    fn new(cx: &App) -> Self {
+    pub fn new(cx: &App) -> Self {
         let settings_store = SettingsStore::global(cx);
         let mut tree = vec![];
         let mut root_entry_indices = vec![];
@@ -269,6 +271,62 @@ impl SettingsUiTree {
             active_entry_index,
         }
     }
+
+    // todo(settings_ui): Make sure `Item::None` paths are added to the paths tree,
+    // so that we can keep none/skip and still test in CI that all settings have
+    #[cfg(feature = "test-support")]
+    pub fn all_paths(&self, cx: &App) -> Vec<Vec<&'static str>> {
+        fn all_paths_rec(
+            tree: &[UiEntry],
+            paths: &mut Vec<Vec<&'static str>>,
+            current_path: &mut Vec<&'static str>,
+            idx: usize,
+            cx: &App,
+        ) {
+            let child = &tree[idx];
+            let mut pushed_path = false;
+            if let Some(path) = child.path.as_ref() {
+                current_path.push(path);
+                paths.push(current_path.clone());
+                pushed_path = true;
+            }
+            // todo(settings_ui): handle dynamic nodes here
+            let selected_descendant_index = child
+                .select_descendant
+                .map(|select_descendant| {
+                    read_settings_value_from_path(
+                        SettingsStore::global(cx).raw_default_settings(),
+                        &current_path,
+                    )
+                    .map(|value| select_descendant(value, cx))
+                })
+                .and_then(|selected_descendant_index| {
+                    selected_descendant_index.map(|index| child.nth_descendant_index(tree, index))
+                });
+
+            if let Some(selected_descendant_index) = selected_descendant_index {
+                // just silently fail if we didn't find a setting value for the path
+                if let Some(descendant_index) = selected_descendant_index {
+                    all_paths_rec(tree, paths, current_path, descendant_index, cx);
+                }
+            } else if let Some(desc_idx) = child.first_descendant_index() {
+                let mut desc_idx = Some(desc_idx);
+                while let Some(descendant_index) = desc_idx {
+                    all_paths_rec(&tree, paths, current_path, descendant_index, cx);
+                    desc_idx = tree[descendant_index].next_sibling;
+                }
+            }
+            if pushed_path {
+                current_path.pop();
+            }
+        }
+
+        let mut paths = Vec::new();
+        for &index in &self.root_entry_indices {
+            all_paths_rec(&self.entries, &mut paths, &mut Vec::new(), index, cx);
+        }
+        paths
+    }
 }
 
 fn render_nav(tree: &SettingsUiTree, _window: &mut Window, cx: &mut Context<SettingsPage>) -> Div {
@@ -444,9 +502,9 @@ fn render_item_single(
     }
 }
 
-fn read_settings_value_from_path<'a>(
+pub fn read_settings_value_from_path<'a>(
     settings_contents: &'a serde_json::Value,
-    path: &[&'static str],
+    path: &[&str],
 ) -> Option<&'a serde_json::Value> {
     // todo(settings_ui) make non recursive, and move to `settings` alongside SettingsValue, and add method to SettingsValue to get nested
     let Some((key, remaining)) = path.split_first() else {

crates/zed/Cargo.toml 🔗

@@ -188,6 +188,7 @@ itertools.workspace = true
 language = { workspace = true, features = ["test-support"] }
 pretty_assertions.workspace = true
 project = { workspace = true, features = ["test-support"] }
+settings_ui = { workspace = true, features = ["test-support"] }
 terminal_view = { workspace = true, features = ["test-support"] }
 tree-sitter-md.workspace = true
 tree-sitter-rust.workspace = true

crates/zed/src/zed.rs 🔗

@@ -4855,4 +4855,34 @@ mod tests {
             "BUG FOUND: Project settings were overwritten when opening via command - original custom content was lost"
         );
     }
+
+    #[gpui::test]
+    fn test_settings_defaults(cx: &mut TestAppContext) {
+        cx.update(|cx| {
+            settings::init(cx);
+            workspace::init_settings(cx);
+            title_bar::init(cx);
+            editor::init_settings(cx);
+            debugger_ui::init(cx);
+        });
+        let default_json =
+            cx.read(|cx| cx.global::<SettingsStore>().raw_default_settings().clone());
+
+        let all_paths = cx.read(|cx| settings_ui::SettingsUiTree::new(cx).all_paths(cx));
+        let mut failures = Vec::new();
+        for path in all_paths {
+            if settings_ui::read_settings_value_from_path(&default_json, &path).is_none() {
+                failures.push(path);
+            }
+        }
+        if !failures.is_empty() {
+            panic!(
+                "No default value found for paths: {:#?}",
+                failures
+                    .into_iter()
+                    .map(|path| path.join("."))
+                    .collect::<Vec<_>>()
+            );
+        }
+    }
 }