Skip to content

Commit 301623f

Browse files
Xclaude
authored andcommitted
feat(core): revision coverage, schema version, tests, CI
Revision coverage (gap 1 in gaps.md): - Make bump_surface_revision() pub so all write paths can reach it - Call bump_surface_revision() after every successful ModulesConfig::save() in ffi.rs (set_module_enabled, set_module_enabled_with_requirements) and Desktop CLI config_cmds.rs (modules_set, modules_reset) - surface.patch already bumped after its save; all paths now consistent Surface snapshot extra: - Add schema_version: 1 to SurfaceSnapshot.extra for forward-compat Tests (33 tests, 0 failures): - config/modules.rs: 15 tests covering ModulesConfig default state, enable/disable enforcement, transitive dep resolution, migration, unknown module errors, shell-motd dep chain - modules/surface.rs: 7 tests covering parse_surface_snapshot round-trip, invalid JSON fallback, navigation_registry extraction, patch_json helpers, SurfacePatch::ModulesSet deserialization - navigation.rs: 11 tests covering JSON round-trip, page/group lookup, registry integrity (required_module -> MODULE_REGISTRY, group pages -> PAGE_DEFINITIONS, defaults exist, global pages valid) CI: - core-tests.yml: cargo test -p arcadia-core on push/PR to dev/main/stable - ffi-drift.yml: builds arcadia-core for aarch64-apple-ios, regenerates Swift bindings, fails if Generated/ drifts from ffi.rs Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
1 parent aa1cd68 commit 301623f

7 files changed

Lines changed: 469 additions & 14 deletions

File tree

.github/workflows/core-tests.yml

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,33 @@
1+
name: Core Tests
2+
3+
on:
4+
push:
5+
branches:
6+
- development
7+
- main
8+
- stable
9+
pull_request:
10+
branches:
11+
- development
12+
- main
13+
- stable
14+
15+
jobs:
16+
test:
17+
name: arcadia-core tests
18+
runs-on: ubuntu-latest
19+
20+
steps:
21+
- name: Checkout repository
22+
uses: actions/checkout@v4
23+
24+
- name: Install Rust toolchain
25+
uses: dtolnay/rust-toolchain@stable
26+
27+
- name: Cache Rust dependencies
28+
uses: swatinem/rust-cache@v2
29+
with:
30+
workspaces: Shared -> target
31+
32+
- name: Run core tests
33+
run: cargo test -p arcadia-core --manifest-path Shared/Cargo.toml

.github/workflows/ffi-drift.yml

Lines changed: 95 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,95 @@
1+
name: FFI Drift Check
2+
3+
on:
4+
push:
5+
branches:
6+
- development
7+
- main
8+
- stable
9+
paths:
10+
- 'Shared/ArcadiaCore/src/ffi.rs'
11+
- 'Shared/ArcadiaCore/src/**.rs'
12+
- 'Mobile/iOS/ArcadiaCore/Generated/**'
13+
pull_request:
14+
branches:
15+
- development
16+
- main
17+
- stable
18+
paths:
19+
- 'Shared/ArcadiaCore/src/ffi.rs'
20+
- 'Shared/ArcadiaCore/src/**.rs'
21+
- 'Mobile/iOS/ArcadiaCore/Generated/**'
22+
workflow_dispatch:
23+
24+
jobs:
25+
ffi-drift:
26+
name: Check FFI bindings drift
27+
runs-on: macos-latest
28+
29+
steps:
30+
- name: Checkout repository
31+
uses: actions/checkout@v4
32+
33+
- name: Install Rust toolchain
34+
uses: dtolnay/rust-toolchain@stable
35+
36+
- name: Install iOS target
37+
run: rustup target add aarch64-apple-ios
38+
39+
- name: Cache Rust dependencies
40+
uses: swatinem/rust-cache@v2
41+
with:
42+
workspaces: Shared -> target
43+
44+
- name: Build arcadia-core for iOS device
45+
run: |
46+
cd Shared
47+
cargo build -p arcadia-core --release --target aarch64-apple-ios
48+
49+
- name: Generate Swift bindings into temp dir
50+
run: |
51+
mkdir -p /tmp/ffi-generated
52+
cd Shared
53+
cargo run -p uniffi-bindgen -- generate \
54+
--library target/aarch64-apple-ios/release/libarcadia_core.a \
55+
--language swift \
56+
--out-dir /tmp/ffi-generated
57+
58+
- name: Diff generated bindings against checked-in
59+
run: |
60+
CHECKED_SWIFT="Mobile/iOS/ArcadiaCore/Generated/arcadia_core.swift"
61+
GENERATED_SWIFT="/tmp/ffi-generated/arcadia_core.swift"
62+
63+
# Detect header filename (uniffi may name it arcadia_coreFFI.h or arcadia_coreCFFI.h)
64+
GENERATED_H=$(ls /tmp/ffi-generated/*.h 2>/dev/null | head -1)
65+
CHECKED_H=""
66+
if [[ -n "$GENERATED_H" ]]; then
67+
HEADER_BASENAME=$(basename "$GENERATED_H")
68+
CHECKED_H="Mobile/iOS/ArcadiaCore/Generated/$HEADER_BASENAME"
69+
fi
70+
71+
DRIFT=0
72+
73+
if ! diff -q "$GENERATED_SWIFT" "$CHECKED_SWIFT" > /dev/null 2>&1; then
74+
echo "::error::Swift bindings drift detected in $CHECKED_SWIFT"
75+
diff "$GENERATED_SWIFT" "$CHECKED_SWIFT" || true
76+
DRIFT=1
77+
fi
78+
79+
if [[ -n "$CHECKED_H" ]] && [[ -f "$CHECKED_H" ]]; then
80+
if ! diff -q "$GENERATED_H" "$CHECKED_H" > /dev/null 2>&1; then
81+
echo "::error::FFI header drift detected in $CHECKED_H"
82+
diff "$GENERATED_H" "$CHECKED_H" || true
83+
DRIFT=1
84+
fi
85+
fi
86+
87+
if [[ "$DRIFT" -eq 1 ]]; then
88+
echo ""
89+
echo "FFI bindings are out of sync. Run:"
90+
echo " bash Shared/Scripts/build-ios-framework.sh"
91+
echo "and commit the updated Generated/ directory."
92+
exit 1
93+
fi
94+
95+
echo "FFI bindings match checked-in version."

Desktop/src/cli/config_cmds.rs

Lines changed: 11 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -178,14 +178,18 @@ fn modules_set(key: &str, value: &str) -> Result<(), String> {
178178
_ => return Err("Module value must be true or false".to_string()),
179179
};
180180
cfg.set_module_state(key, parsed)?;
181-
cfg.save().map_err(|err| err.to_string())
181+
cfg.save().map_err(|err| err.to_string())?;
182+
arcadia_core::modules::surface::bump_surface_revision();
183+
Ok(())
182184
}
183185

184186
fn modules_reset(target: Option<&str>) -> Result<(), String> {
185187
match target {
186-
None => ModulesConfig::default()
187-
.save()
188-
.map_err(|err| err.to_string()),
188+
None => {
189+
ModulesConfig::default().save().map_err(|err| err.to_string())?;
190+
arcadia_core::modules::surface::bump_surface_revision();
191+
Ok(())
192+
}
189193
Some(key) => {
190194
let defaults = ModulesConfig::default();
191195
let default_value = defaults
@@ -195,7 +199,9 @@ fn modules_reset(target: Option<&str>) -> Result<(), String> {
195199
.ok_or_else(|| "Unknown module key".to_string())?;
196200
let mut cfg = ModulesConfig::load_or_create().map_err(|err| err.to_string())?;
197201
cfg.set_module_state(key, default_value)?;
198-
cfg.save().map_err(|err| err.to_string())
202+
cfg.save().map_err(|err| err.to_string())?;
203+
arcadia_core::modules::surface::bump_surface_revision();
204+
Ok(())
199205
}
200206
}
201207
}

Shared/ArcadiaCore/src/config/modules.rs

Lines changed: 138 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -229,3 +229,141 @@ impl ConfigFile for ModulesConfig {
229229
changed
230230
}
231231
}
232+
233+
#[cfg(test)]
234+
mod tests {
235+
use super::*;
236+
237+
fn base() -> ModulesConfig {
238+
ModulesConfig::default()
239+
}
240+
241+
#[test]
242+
fn default_surface_enabled_others_disabled() {
243+
let cfg = base();
244+
assert_eq!(cfg.modules.get(SURFACE_MODULE_NAME), Some(&true));
245+
assert_eq!(cfg.modules.get(SHELL_MODULE_NAME), Some(&false));
246+
assert_eq!(cfg.modules.get(NET_MODULE_NAME), Some(&false));
247+
assert_eq!(cfg.modules.get(LAN_MODULE_NAME), Some(&false));
248+
}
249+
250+
#[test]
251+
fn set_module_state_enables_known_module() {
252+
let mut cfg = base();
253+
cfg.set_module_state(SHELL_MODULE_NAME, true).unwrap();
254+
assert_eq!(cfg.modules.get(SHELL_MODULE_NAME), Some(&true));
255+
}
256+
257+
#[test]
258+
fn set_module_state_blocks_enable_when_dep_missing() {
259+
let mut cfg = base();
260+
// lan requires net; net is disabled
261+
let err = cfg.set_module_state(LAN_MODULE_NAME, true).unwrap_err();
262+
assert!(err.contains("net"), "error should mention missing dep: {err}");
263+
}
264+
265+
#[test]
266+
fn set_module_state_blocks_disable_when_dependent_enabled() {
267+
let mut cfg = base();
268+
cfg.set_module_state(NET_MODULE_NAME, true).unwrap();
269+
cfg.set_module_state(LAN_MODULE_NAME, true).unwrap();
270+
let err = cfg.set_module_state(NET_MODULE_NAME, false).unwrap_err();
271+
assert!(err.contains("lan"), "error should mention blocking dependent: {err}");
272+
}
273+
274+
#[test]
275+
fn enable_with_requirements_transitively_enables_deps() {
276+
let mut cfg = base();
277+
cfg.enable_with_requirements(LAN_MODULE_NAME).unwrap();
278+
assert_eq!(cfg.modules.get(NET_MODULE_NAME), Some(&true));
279+
assert_eq!(cfg.modules.get(LAN_MODULE_NAME), Some(&true));
280+
}
281+
282+
#[test]
283+
fn enable_with_requirements_remote_session_enables_net_and_lan() {
284+
let mut cfg = base();
285+
cfg.enable_with_requirements(REMOTE_SESSION_MODULE_NAME).unwrap();
286+
assert_eq!(cfg.modules.get(NET_MODULE_NAME), Some(&true));
287+
assert_eq!(cfg.modules.get(LAN_MODULE_NAME), Some(&true));
288+
assert_eq!(cfg.modules.get(REMOTE_SESSION_MODULE_NAME), Some(&true));
289+
}
290+
291+
#[test]
292+
fn missing_requirements_for_lan_without_net() {
293+
let cfg = base();
294+
let missing = cfg.missing_requirements_for(LAN_MODULE_NAME).unwrap();
295+
assert!(missing.contains(&NET_MODULE_NAME.to_string()));
296+
}
297+
298+
#[test]
299+
fn missing_requirements_empty_when_dep_met() {
300+
let mut cfg = base();
301+
cfg.set_module_state(NET_MODULE_NAME, true).unwrap();
302+
let missing = cfg.missing_requirements_for(LAN_MODULE_NAME).unwrap();
303+
assert!(missing.is_empty());
304+
}
305+
306+
#[test]
307+
fn missing_requirements_unknown_module_errors() {
308+
let cfg = base();
309+
assert!(cfg.missing_requirements_for("does-not-exist").is_err());
310+
}
311+
312+
#[test]
313+
fn merge_defaults_migrates_legacy_lan_key() {
314+
let mut cfg = ModulesConfig {
315+
modules: {
316+
let mut m = std::collections::BTreeMap::new();
317+
m.insert(LEGACY_LAN_MODULE_NAME.to_string(), true);
318+
m
319+
},
320+
};
321+
let changed = cfg.merge_defaults();
322+
assert!(changed);
323+
assert!(!cfg.modules.contains_key(LEGACY_LAN_MODULE_NAME));
324+
assert_eq!(cfg.modules.get(LAN_MODULE_NAME), Some(&true));
325+
}
326+
327+
#[test]
328+
fn merge_defaults_adds_missing_modules() {
329+
let mut cfg = ModulesConfig { modules: std::collections::BTreeMap::new() };
330+
let changed = cfg.merge_defaults();
331+
assert!(changed);
332+
for manifest in MODULE_REGISTRY {
333+
assert!(
334+
cfg.modules.contains_key(manifest.name),
335+
"merge_defaults must add missing module '{}'",
336+
manifest.name
337+
);
338+
}
339+
}
340+
341+
#[test]
342+
fn manifest_for_known_module() {
343+
let m = ModulesConfig::manifest_for(SHELL_MODULE_NAME).unwrap();
344+
assert_eq!(m.name, SHELL_MODULE_NAME);
345+
}
346+
347+
#[test]
348+
fn manifest_for_unknown_returns_none() {
349+
assert!(ModulesConfig::manifest_for("totally-fake").is_none());
350+
}
351+
352+
#[test]
353+
fn set_module_state_unknown_module_errors() {
354+
let mut cfg = base();
355+
assert!(cfg.set_module_state("ghost-module", true).is_err());
356+
}
357+
358+
#[test]
359+
fn shell_motd_requires_shell() {
360+
let mut cfg = base();
361+
// shell-motd requires shell; shell disabled → enable should fail
362+
let err = cfg.set_module_state(SHELL_MOTD_MODULE_NAME, true).unwrap_err();
363+
assert!(err.contains("shell"), "error should mention shell: {err}");
364+
// enable shell first, then motd should work
365+
cfg.set_module_state(SHELL_MODULE_NAME, true).unwrap();
366+
cfg.set_module_state(SHELL_MOTD_MODULE_NAME, true).unwrap();
367+
assert_eq!(cfg.modules.get(SHELL_MOTD_MODULE_NAME), Some(&true));
368+
}
369+
}

Shared/ArcadiaCore/src/ffi.rs

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -101,10 +101,10 @@ pub fn set_module_enabled(name: String, enabled: bool) -> String {
101101
let result = cfg.set_module_state(&name, enabled);
102102
match result {
103103
Ok(()) => match cfg.save() {
104-
Ok(()) => format!(
105-
"Module {name} {}",
106-
if enabled { "enabled" } else { "disabled" }
107-
),
104+
Ok(()) => {
105+
modules::surface::bump_surface_revision();
106+
format!("Module {name} {}", if enabled { "enabled" } else { "disabled" })
107+
}
108108
Err(e) => format!("Error saving config: {e}"),
109109
},
110110
Err(e) => e,
@@ -124,10 +124,10 @@ pub fn set_module_enabled_with_requirements(name: String, enabled: bool) -> Stri
124124
};
125125
match result {
126126
Ok(()) => match cfg.save() {
127-
Ok(()) => format!(
128-
"Module {name} {}",
129-
if enabled { "enabled" } else { "disabled" }
130-
),
127+
Ok(()) => {
128+
modules::surface::bump_surface_revision();
129+
format!("Module {name} {}", if enabled { "enabled" } else { "disabled" })
130+
}
131131
Err(e) => format!("Error saving config: {e}"),
132132
},
133133
Err(e) => e,

0 commit comments

Comments
 (0)