[pve-devel] [PATCH installer 0/7] tui: add min/max constraints for bootdisk parameters

[pve-devel] [PATCH installer 0/7] tui: add min/max constraints for bootdisk parameters

[Christoph Heiss]

This adds minimum and maximum value constraints to the LVM, Btrfs and
ZFS parameters in the bootdisk dialog. The current behaviour is to to
either silently discard such values, which might yield unexpected
results to users, or to throw some error later on during installation
(due to e.g. failed ZFS pool creation), which does not indicate the
exact cause.

Both cases are really not desirable from a UX standpoint, especially
with e.g. ZFS, where users might not know about the various constraints.

The maximum value is clamped in such a way that the user cannot even
enter a value bigger than allowed (which is the current behavior), but
the same cannot be done for the minimum value. If the allowed range is 9
to 13, one could not enter e.g. `1` first, even if `12` was the desired
value.

Thus, the other approach is to simply validate on the dialog
confirmation, passing down errors. The latter needed some (prep-)work to
convert `Option` -> `Result`. Two new error enums were also introduced,
to better represent errors that occured, rather than fiddeling around
with strings.

The TUI tests introduced in #7 show some techniques how components and
their interactions can be properly tested.

The same can be done for the GUI installer of course, but I didn't want
to include in this series to avoid it being larger-than-necessary.
series.

Christoph Heiss (7):
  tui: fix setting content when using the `DiskSizeEditView` builder
  tui: improve `FormView` error handling
  tui: add optional min-value constraint to `NumericEditView` and
    `DiskSizeEditView`
  tui: add min/max constraints for ZFS bootdisk parameters
  tui: add min/max contraints for LVM bootdisk parameters
  tui: add min/max contraints for Btrfs bootdisk parameters
  tui: views: add some TUI component tests

 proxmox-tui-installer/src/main.rs           |  51 ++-
 proxmox-tui-installer/src/options.rs        |   2 +-
 proxmox-tui-installer/src/views/bootdisk.rs | 151 ++++---
 proxmox-tui-installer/src/views/mod.rs      | 421 ++++++++++++++++----
 proxmox-tui-installer/src/views/timezone.rs | 115 +++++-
 5 files changed, 608 insertions(+), 132 deletions(-)

--
2.41.0

[pve-devel] [PATCH installer 1/7] tui: fix setting content when using the `DiskSizeEditView` builder

[Christoph Heiss]

Previously, it would throw away all other settings (like `max_value`),
if a construct like

  DiskSizeEditView::new().max_value(8.0).content(8.0)

was used, due to simply replacing the inner view.
Instead, modify the inner view.

Signed-off-by: Christoph Heiss <c.heiss@proxmox.com>
---
 proxmox-tui-installer/src/views/mod.rs | 19 ++++++++++++++++---
 1 file changed, 16 insertions(+), 3 deletions(-)

diff --git a/proxmox-tui-installer/src/views/mod.rs b/proxmox-tui-installer/src/views/mod.rs
index aa24fa4..76f96a1 100644
--- a/proxmox-tui-installer/src/views/mod.rs
+++ b/proxmox-tui-installer/src/views/mod.rs
@@ -1,4 +1,4 @@
-use std::{net::IpAddr, rc::Rc, str::FromStr};
+use std::{mem, net::IpAddr, rc::Rc, str::FromStr};
 
 use cursive::{
     event::{Event, EventResult},
@@ -190,8 +190,21 @@ impl DiskSizeEditView {
     }
 
     pub fn content(mut self, content: f64) -> Self {
-        if let Some(view) = self.view.get_child_mut(0).and_then(|v| v.downcast_mut()) {
-            *view = FloatEditView::new().content(content).full_width();
+        if let Some(view) = self
+            .view
+            .get_child_mut(0)
+            .and_then(|v| v.downcast_mut::<ResizedView<FloatEditView>>())
+        {
+            // We need actual ownership here of the inner `FloatEditView` to call `.content()` on
+            // it. Thus first swap it out with a dummy, modify it and swap it back in.
+            // This procedure ensures other settings (like `max_value`) is preserved on the inner
+            // view.
+
+            let mut inner = FloatEditView::new();
+            mem::swap(view.get_inner_mut(), &mut inner);
+
+            inner = inner.content(content);
+            mem::swap(view.get_inner_mut(), &mut inner);
         }
 
         self
-- 
2.42.0

[pve-devel] [PATCH installer 2/7] tui: improve `FormView` error handling

[Christoph Heiss]

Mostly internal changes without any user-visible changes; replaces all
optional return values in form with result that can hold more specific
error causes.

Signed-off-by: Christoph Heiss <c.heiss@proxmox.com>
---
RFC; just a thought: It could make sense to introduce the `anyhow` crate
here in the installer as well. We already use it in PBS, so nothing
completely new and it would simplify error handling quite a bit as well,
I think. But we can also do without it as well, as this series shows.

 proxmox-tui-installer/src/main.rs           |  16 +-
 proxmox-tui-installer/src/views/bootdisk.rs | 105 +++++----
 proxmox-tui-installer/src/views/mod.rs      | 245 +++++++++++++++-----
 proxmox-tui-installer/src/views/timezone.rs |   6 +-
 4 files changed, 259 insertions(+), 113 deletions(-)

diff --git a/proxmox-tui-installer/src/main.rs b/proxmox-tui-installer/src/main.rs
index 3f01713..ab990a8 100644
--- a/proxmox-tui-installer/src/main.rs
+++ b/proxmox-tui-installer/src/main.rs
@@ -499,15 +499,15 @@ fn password_dialog(siv: &mut Cursive) -> InstallerView {
             let options = siv.call_on_name("password-options", |view: &mut FormView| {
                 let root_password = view
                     .get_value::<EditView, _>(0)
-                    .ok_or("failed to retrieve password")?;
+                    .map_err(|_| "failed to retrieve password")?;

                 let confirm_password = view
                     .get_value::<EditView, _>(1)
-                    .ok_or("failed to retrieve password confirmation")?;
+                    .map_err(|_| "failed to retrieve password confirmation")?;

                 let email = view
                     .get_value::<EditView, _>(2)
-                    .ok_or("failed to retrieve email")?;
+                    .map_err(|_| "failed to retrieve email")?;

                 let email_regex =
                     Regex::new(r"^[\w\+\-\~]+(\.[\w\+\-\~]+)*@[a-zA-Z0-9\-]+(\.[a-zA-Z0-9\-]+)*$")
@@ -588,27 +588,27 @@ fn network_dialog(siv: &mut Cursive) -> InstallerView {
             let options = siv.call_on_name("network-options", |view: &mut FormView| {
                 let ifname = view
                     .get_value::<SelectView, _>(0)
-                    .ok_or("failed to retrieve management interface name")?;
+                    .map_err(|_| "failed to retrieve management interface name")?;

                 let fqdn = view
                     .get_value::<EditView, _>(1)
-                    .ok_or("failed to retrieve host FQDN")?
+                    .map_err(|_| "failed to retrieve host FQDN")?
                     .parse::<Fqdn>()
                     .map_err(|err| format!("hostname does not look valid:\n\n{err}"))?;

                 let address = view
                     .get_value::<CidrAddressEditView, _>(2)
-                    .ok_or("failed to retrieve host address")?;
+                    .map_err(|_| "failed to retrieve host address")?;

                 let gateway = view
                     .get_value::<EditView, _>(3)
-                    .ok_or("failed to retrieve gateway address")?
+                    .map_err(|_| "failed to retrieve gateway address")?
                     .parse::<IpAddr>()
                     .map_err(|err| err.to_string())?;

                 let dns_server = view
                     .get_value::<EditView, _>(4)
-                    .ok_or("failed to retrieve DNS server address")?
+                    .map_err(|_| "failed to retrieve DNS server address")?
                     .parse::<IpAddr>()
                     .map_err(|err| err.to_string())?;

diff --git a/proxmox-tui-installer/src/views/bootdisk.rs b/proxmox-tui-installer/src/views/bootdisk.rs
index dbd13ea..46bdd9f 100644
--- a/proxmox-tui-installer/src/views/bootdisk.rs
+++ b/proxmox-tui-installer/src/views/bootdisk.rs
@@ -75,8 +75,11 @@ impl BootdiskOptionsView {
                 .get_child_mut(0)
                 .and_then(|v| v.downcast_mut::<NamedView<FormView>>())
                 .map(NamedView::<FormView>::get_mut)
-                .and_then(|v| v.get_value::<SelectView<Disk>, _>(0))
-                .ok_or("failed to retrieve bootdisk")?;
+                .ok_or("failed go retrieve disk selection view")
+                .and_then(|v| {
+                    v.get_value::<SelectView<Disk>, _>(0)
+                        .map_err(|_| "failed to retrieve bootdisk")
+                })?;

             options.disks = vec![disk];
         }
@@ -181,8 +184,9 @@ impl AdvancedBootdiskOptionsView {
             .view
             .get_child(1)
             .and_then(|v| v.downcast_ref::<FormView>())
-            .and_then(|v| v.get_value::<SelectView<FsType>, _>(0))
-            .ok_or("Failed to retrieve filesystem type".to_owned())?;
+            .ok_or("Failed to retrieve advanced bootdisk options view")?
+            .get_value::<SelectView<FsType>, _>(0)
+            .map_err(|_| "Failed to retrieve filesystem type".to_owned())?;

         let advanced = self
             .view
@@ -190,21 +194,15 @@ impl AdvancedBootdiskOptionsView {
             .ok_or("Failed to retrieve advanced bootdisk options view".to_owned())?;

         if let Some(view) = advanced.downcast_mut::<LvmBootdiskOptionsView>() {
-            let advanced = view
-                .get_values()
-                .map(AdvancedBootdiskOptions::Lvm)
-                .ok_or("Failed to retrieve advanced bootdisk options")?;
+            let advanced = view.get_values()?;

             Ok(BootdiskOptions {
                 disks: vec![],
                 fstype,
-                advanced,
+                advanced: AdvancedBootdiskOptions::Lvm(advanced),
             })
         } else if let Some(view) = advanced.downcast_mut::<ZfsBootdiskOptionsView>() {
-            let (disks, advanced) = view
-                .get_values()
-                .ok_or("Failed to retrieve advanced bootdisk options")?;
-
+            let (disks, advanced) = view.get_values()?;
             if let FsType::Zfs(level) = fstype {
                 check_zfs_raid_config(level, &disks).map_err(|err| format!("{fstype}: {err}"))?;
             }
@@ -215,9 +213,7 @@ impl AdvancedBootdiskOptionsView {
                 advanced: AdvancedBootdiskOptions::Zfs(advanced),
             })
         } else if let Some(view) = advanced.downcast_mut::<BtrfsBootdiskOptionsView>() {
-            let (disks, advanced) = view
-                .get_values()
-                .ok_or("Failed to retrieve advanced bootdisk options")?;
+            let (disks, advanced) = view.get_values()?;

             if let FsType::Btrfs(level) = fstype {
                 check_btrfs_raid_config(level, &disks).map_err(|err| format!("{fstype}: {err}"))?;
@@ -275,25 +271,34 @@ impl LvmBootdiskOptionsView {
         Self { view }
     }

-    fn get_values(&mut self) -> Option<LvmBootdiskOptions> {
+    fn get_values(&mut self) -> Result<LvmBootdiskOptions, String> {
         let is_pve = crate::setup_info().config.product == ProxmoxProduct::PVE;
         let min_lvm_free_id = if is_pve { 4 } else { 2 };
+
+        let total_size = self.view.get_value::<DiskSizeEditView, _>(0)?;
+        let swap_size = self.view.get_opt_value::<DiskSizeEditView, _>(1)?;
+
         let max_root_size = if is_pve {
-            self.view.get_value::<DiskSizeEditView, _>(2)
+            self.view.get_opt_value::<DiskSizeEditView, _>(2)?
         } else {
             None
         };
         let max_data_size = if is_pve {
-            self.view.get_value::<DiskSizeEditView, _>(3)
+            self.view.get_opt_value::<DiskSizeEditView, _>(3)?
         } else {
             None
         };
-        Some(LvmBootdiskOptions {
-            total_size: self.view.get_value::<DiskSizeEditView, _>(0)?,
-            swap_size: self.view.get_value::<DiskSizeEditView, _>(1),
+
+        let min_lvm_free = self
+            .view
+            .get_opt_value::<DiskSizeEditView, _>(min_lvm_free_id)?;
+
+        Ok(LvmBootdiskOptions {
+            total_size,
+            swap_size,
             max_root_size,
             max_data_size,
-            min_lvm_free: self.view.get_value::<DiskSizeEditView, _>(min_lvm_free_id),
+            min_lvm_free,
         })
     }
 }
@@ -405,7 +410,7 @@ impl<T: View> MultiDiskOptionsView<T> {
         let mut selected_disks = Vec::new();

         for i in 0..disk_form.len() {
-            let disk = disk_form.get_value::<SelectView<Option<Disk>>, _>(i)?;
+            let disk = disk_form.get_value::<SelectView<Option<Disk>>, _>(i).ok()?;

             // `None` means no disk was selected for this slot
             if let Some(disk) = disk {
@@ -462,11 +467,19 @@ impl BtrfsBootdiskOptionsView {
         Self { view }
     }

-    fn get_values(&mut self) -> Option<(Vec<Disk>, BtrfsBootdiskOptions)> {
-        let (disks, selected_disks) = self.view.get_disks_and_selection()?;
-        let disk_size = self.view.inner_mut()?.get_value::<DiskSizeEditView, _>(0)?;
+    fn get_values(&mut self) -> Result<(Vec<Disk>, BtrfsBootdiskOptions), String> {
+        let (disks, selected_disks) = self
+            .view
+            .get_disks_and_selection()
+            .ok_or("failed to retrieve disk selection".to_owned())?;
+
+        let disk_size = self
+            .view
+            .inner_mut()
+            .ok_or("failed to retrieve Btrfs disk size")?
+            .get_value::<DiskSizeEditView, _>(0)?;

-        Some((
+        Ok((
             disks,
             BtrfsBootdiskOptions {
                 disk_size,
@@ -524,17 +537,31 @@ impl ZfsBootdiskOptionsView {
         Self { view }
     }

-    fn get_values(&mut self) -> Option<(Vec<Disk>, ZfsBootdiskOptions)> {
-        let (disks, selected_disks) = self.view.get_disks_and_selection()?;
-        let view = self.view.inner_mut()?;
-
-        let ashift = view.get_value::<IntegerEditView, _>(0)?;
-        let compress = view.get_value::<SelectView<_>, _>(1)?;
-        let checksum = view.get_value::<SelectView<_>, _>(2)?;
-        let copies = view.get_value::<IntegerEditView, _>(3)?;
-        let disk_size = view.get_value::<DiskSizeEditView, _>(4)?;
-
-        Some((
+    fn get_values(&mut self) -> Result<(Vec<Disk>, ZfsBootdiskOptions), String> {
+        let (disks, selected_disks) = self
+            .view
+            .get_disks_and_selection()
+            .ok_or("failed to retrieve disk selection".to_owned())?;
+
+        let view = self.view.inner_mut().ok_or("failed to retrieve view")?;
+
+        let ashift = view
+            .get_value::<IntegerEditView, _>(0)
+            .map_err(|err| format!("invalid ashift value: {err}"))?;
+        let compress = view
+            .get_value::<SelectView<_>, _>(1)
+            .map_err(|_| "failed to get compress option")?;
+        let checksum = view
+            .get_value::<SelectView<_>, _>(2)
+            .map_err(|_| "failed to get checksum option")?;
+        let copies = view
+            .get_value::<IntegerEditView, _>(3)
+            .map_err(|err| format!("invalid copies value: {err}"))?;
+        let disk_size = view
+            .get_value::<DiskSizeEditView, _>(4)
+            .map_err(|err| format!("invalid disk size: {err}"))?;
+
+        Ok((
             disks,
             ZfsBootdiskOptions {
                 ashift,
diff --git a/proxmox-tui-installer/src/views/mod.rs b/proxmox-tui-installer/src/views/mod.rs
index 76f96a1..7efd487 100644
--- a/proxmox-tui-installer/src/views/mod.rs
+++ b/proxmox-tui-installer/src/views/mod.rs
@@ -1,4 +1,4 @@
-use std::{mem, net::IpAddr, rc::Rc, str::FromStr};
+use std::{fmt, mem, net::IpAddr, rc::Rc, str::FromStr};

 use cursive::{
     event::{Event, EventResult},
@@ -18,6 +18,46 @@ pub use table_view::*;
 mod timezone;
 pub use timezone::*;

+pub enum NumericEditViewError<T>
+where
+    T: FromStr,
+{
+    ParseError(<T as FromStr>::Err),
+    OutOfRange {
+        value: T,
+        min: Option<T>,
+        max: Option<T>,
+    },
+    Empty,
+    InvalidView,
+}
+
+impl<T> fmt::Display for NumericEditViewError<T>
+where
+    T: fmt::Display + FromStr,
+    <T as FromStr>::Err: fmt::Display,
+{
+    fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
+        use NumericEditViewError::*;
+        match self {
+            ParseError(err) => write!(f, "failed to parse value: {err}"),
+            OutOfRange { value, min, max } => {
+                write!(f, "out of range: {value}")?;
+                match (min, max) {
+                    (Some(min), Some(max)) => {
+                        write!(f, ", must be at least {min} and at most {max}")
+                    }
+                    (Some(min), None) => write!(f, ", must be at least {min}"),
+                    (None, Some(max)) => write!(f, ", must be at most {max}"),
+                    _ => Ok(()),
+                }
+            }
+            Empty => write!(f, "value required"),
+            InvalidView => write!(f, "invalid view state"),
+        }
+    }
+}
+
 pub struct NumericEditView<T> {
     view: EditView,
     max_value: Option<T>,
@@ -25,7 +65,10 @@ pub struct NumericEditView<T> {
     allow_empty: bool,
 }

-impl<T: Copy + ToString + FromStr + PartialOrd> NumericEditView<T> {
+impl<T> NumericEditView<T>
+where
+    T: Copy + ToString + FromStr + PartialOrd,
+{
     pub fn new() -> Self {
         Self {
             view: EditView::new().content("0"),
@@ -35,6 +78,15 @@ impl<T: Copy + ToString + FromStr + PartialOrd> NumericEditView<T> {
         }
     }

+    pub fn new_empty() -> Self {
+        Self {
+            view: EditView::new(),
+            max_value: None,
+            max_content_width: None,
+            allow_empty: true,
+        }
+    }
+
     pub fn max_value(mut self, max: T) -> Self {
         self.max_value = Some(max);
         self
@@ -46,30 +98,18 @@ impl<T: Copy + ToString + FromStr + PartialOrd> NumericEditView<T> {
         self
     }

-    pub fn allow_empty(mut self, value: bool) -> Self {
-        self.allow_empty = value;
-
-        if value {
-            self.view = EditView::new();
-        } else {
-            self.view = EditView::new().content("0");
-        }
-
-        self.view.set_max_content_width(self.max_content_width);
-        self
-    }
-
-    pub fn get_content(&self) -> Result<T, <T as FromStr>::Err> {
-        assert!(!self.allow_empty);
-        self.view.get_content().parse()
-    }
-
-    pub fn get_content_maybe(&self) -> Option<Result<T, <T as FromStr>::Err>> {
+    pub fn get_content(&self) -> Result<T, NumericEditViewError<T>> {
         let content = self.view.get_content();
-        if !content.is_empty() {
-            Some(self.view.get_content().parse())
-        } else {
-            None
+
+        match content.parse() {
+            Err(_) if content.is_empty() && self.allow_empty => Err(NumericEditViewError::Empty),
+            Err(err) => Err(NumericEditViewError::ParseError(err)),
+            Ok(value) if !self.in_range(value) => Err(NumericEditViewError::OutOfRange {
+                value,
+                min: self.min_value,
+                max: self.max_value,
+            }),
+            Ok(value) => Ok(value),
         }
     }

@@ -77,6 +117,10 @@ impl<T: Copy + ToString + FromStr + PartialOrd> NumericEditView<T> {
         self.max_value = Some(max);
     }

+    fn in_range(&self, value: T) -> bool {
+        !self.max_value.map_or(false, |max| value >= max)
+    }
+
     fn check_bounds(&mut self, original: Rc<String>, result: EventResult) -> EventResult {
         // Check if the new value is actually valid according to the max value, if set
         if let Some(max) = self.max_value {
@@ -163,7 +207,6 @@ impl IntegerEditView {

 pub struct DiskSizeEditView {
     view: LinearLayout,
-    allow_empty: bool,
 }

 impl DiskSizeEditView {
@@ -172,21 +215,15 @@ impl DiskSizeEditView {
             .child(FloatEditView::new().full_width())
             .child(TextView::new(" GB"));

-        Self {
-            view,
-            allow_empty: false,
-        }
+        Self { view }
     }

     pub fn new_emptyable() -> Self {
         let view = LinearLayout::horizontal()
-            .child(FloatEditView::new().allow_empty(true).full_width())
+            .child(FloatEditView::new_empty().full_width())
             .child(TextView::new(" GB"));

-        Self {
-            view,
-            allow_empty: true,
-        }
+        Self { view }
     }

     pub fn content(mut self, content: f64) -> Self {
@@ -230,20 +267,19 @@ impl DiskSizeEditView {
         self
     }

-    pub fn get_content(&self) -> Option<f64> {
-        self.with_view(|v| {
-            v.get_child(0)?
-                .downcast_ref::<ResizedView<FloatEditView>>()?
-                .with_view(|v| {
-                    if self.allow_empty {
-                        v.get_content_maybe().and_then(Result::ok)
-                    } else {
-                        v.get_content().ok()
-                    }
-                })
-                .flatten()
-        })
-        .flatten()
+    pub fn get_content(&self) -> Result<f64, NumericEditViewError<f64>> {
+        let content = self
+            .with_view(|v| {
+                v.get_child(0)?
+                    .downcast_ref::<ResizedView<FloatEditView>>()?
+                    .with_view(|v| v.get_content())
+            })
+            .flatten();
+
+        match content {
+            Some(res) => res,
+            None => Err(NumericEditViewError::InvalidView),
+        }
     }
 }

@@ -252,18 +288,28 @@ impl ViewWrapper for DiskSizeEditView {
 }

 pub trait FormViewGetValue<R> {
-    fn get_value(&self) -> Option<R>;
+    type Error;
+
+    fn get_value(&self) -> Result<R, Self::Error>;
+
+    fn get_opt_value(&self) -> Result<Option<R>, Self::Error> {
+        self.get_value().map(|val| Some(val))
+    }
 }

 impl FormViewGetValue<String> for EditView {
-    fn get_value(&self) -> Option<String> {
-        Some((*self.get_content()).clone())
+    type Error = ();
+
+    fn get_value(&self) -> Result<String, Self::Error> {
+        Ok((*self.get_content()).clone())
     }
 }

 impl<T: 'static + Clone> FormViewGetValue<T> for SelectView<T> {
-    fn get_value(&self) -> Option<T> {
-        self.selection().map(|v| (*v).clone())
+    type Error = ();
+
+    fn get_value(&self) -> Result<T, Self::Error> {
+        self.selection().map(|v| (*v).clone()).ok_or(())
     }
 }

@@ -272,14 +318,26 @@ where
     T: Copy + ToString + FromStr + PartialOrd,
     NumericEditView<T>: ViewWrapper,
 {
-    fn get_value(&self) -> Option<T> {
-        self.get_content().ok()
+    type Error = NumericEditViewError<T>;
+
+    fn get_value(&self) -> Result<T, Self::Error> {
+        self.get_content()
+    }
+
+    fn get_opt_value(&self) -> Result<Option<T>, Self::Error> {
+        match self.get_content() {
+            Ok(val) => Ok(Some(val)),
+            Err(NumericEditViewError::Empty) => Ok(None),
+            Err(err) => Err(err),
+        }
     }
 }

 impl FormViewGetValue<CidrAddress> for CidrAddressEditView {
-    fn get_value(&self) -> Option<CidrAddress> {
-        self.get_values()
+    type Error = ();
+
+    fn get_value(&self) -> Result<CidrAddress, Self::Error> {
+        self.get_values().ok_or(())
     }
 }

@@ -289,15 +347,59 @@ where
     NamedView<T>: ViewWrapper,
     <NamedView<T> as ViewWrapper>::V: FormViewGetValue<R>,
 {
-    fn get_value(&self) -> Option<R> {
-        self.with_view(|v| v.get_value()).flatten()
+    type Error = ();
+
+    fn get_value(&self) -> Result<R, Self::Error> {
+        match self.with_view(|v| v.get_value()) {
+            Some(Ok(res)) => Ok(res),
+            _ => Err(()),
+        }
     }
 }

 impl FormViewGetValue<f64> for DiskSizeEditView {
-    fn get_value(&self) -> Option<f64> {
+    type Error = NumericEditViewError<f64>;
+
+    fn get_value(&self) -> Result<f64, Self::Error> {
         self.get_content()
     }
+
+    fn get_opt_value(&self) -> Result<Option<f64>, Self::Error> {
+        match self.get_content() {
+            Ok(val) => Ok(Some(val)),
+            Err(NumericEditViewError::Empty) => Ok(None),
+            Err(err) => Err(err),
+        }
+    }
+}
+
+pub enum FormViewError<T> {
+    ChildNotFound(usize),
+    ValueError(T),
+}
+
+impl<T: fmt::Display> fmt::Display for FormViewError<T> {
+    fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
+        match self {
+            Self::ChildNotFound(i) => write!(f, "child with index {i} does not exist"),
+            Self::ValueError(err) => write!(f, "{err}"),
+        }
+    }
+}
+
+impl<T: fmt::Debug> fmt::Debug for FormViewError<T> {
+    fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
+        match self {
+            Self::ChildNotFound(index) => write!(f, "ChildNotFound({index})"),
+            Self::ValueError(err) => write!(f, "ValueError({err:?})"),
+        }
+    }
+}
+
+impl<T: fmt::Display> From<FormViewError<T>> for String {
+    fn from(value: FormViewError<T>) -> Self {
+        value.to_string()
+    }
 }

 pub struct FormView {
@@ -348,11 +450,28 @@ impl FormView {
             .downcast_mut::<T>()
     }

-    pub fn get_value<T, R>(&self, index: usize) -> Option<R>
+    pub fn get_value<T, R>(
+        &self,
+        index: usize,
+    ) -> Result<R, FormViewError<<T as FormViewGetValue<R>>::Error>>
+    where
+        T: View + FormViewGetValue<R>,
+    {
+        self.get_child::<T>(index)
+            .ok_or(FormViewError::ChildNotFound(index))
+            .and_then(|v| v.get_value().map_err(FormViewError::ValueError))
+    }
+
+    pub fn get_opt_value<T, R>(
+        &self,
+        index: usize,
+    ) -> Result<Option<R>, FormViewError<<T as FormViewGetValue<R>>::Error>>
     where
         T: View + FormViewGetValue<R>,
     {
-        self.get_child::<T>(index)?.get_value()
+        self.get_child::<T>(index)
+            .ok_or(FormViewError::ChildNotFound(index))
+            .and_then(|v| v.get_opt_value().map_err(FormViewError::ValueError))
     }

     pub fn replace_child(&mut self, index: usize, view: impl View) {
diff --git a/proxmox-tui-installer/src/views/timezone.rs b/proxmox-tui-installer/src/views/timezone.rs
index 6732286..bd38a92 100644
--- a/proxmox-tui-installer/src/views/timezone.rs
+++ b/proxmox-tui-installer/src/views/timezone.rs
@@ -100,17 +100,17 @@ impl TimezoneOptionsView {
         let country = self
             .view
             .get_value::<SelectView, _>(0)
-            .ok_or("failed to retrieve timezone")?;
+            .map_err(|_| "failed to retrieve country")?;

         let timezone = self
             .view
             .get_value::<NamedView<SelectView>, _>(1)
-            .ok_or("failed to retrieve timezone")?;
+            .map_err(|_| "failed to retrieve timezone")?;

         let kmap = self
             .view
             .get_value::<SelectView<KeyboardMapping>, _>(2)
-            .ok_or("failed to retrieve keyboard layout")?;
+            .map_err(|_| "failed to retrieve keyboard layout")?;

         Ok(TimezoneOptions {
             country,
--
2.42.0

[pve-devel] [PATCH installer 3/7] tui: add optional min-value constraint to `NumericEditView` and `DiskSizeEditView`

[Christoph Heiss]

Signed-off-by: Christoph Heiss <c.heiss@proxmox.com>
---
 proxmox-tui-installer/src/views/mod.rs | 46 ++++++++++++++++++++++----
 1 file changed, 39 insertions(+), 7 deletions(-)

diff --git a/proxmox-tui-installer/src/views/mod.rs b/proxmox-tui-installer/src/views/mod.rs
index 7efd487..0fe715e 100644
--- a/proxmox-tui-installer/src/views/mod.rs
+++ b/proxmox-tui-installer/src/views/mod.rs
@@ -60,6 +60,7 @@ where
 
 pub struct NumericEditView<T> {
     view: EditView,
+    min_value: Option<T>,
     max_value: Option<T>,
     max_content_width: Option<usize>,
     allow_empty: bool,
@@ -72,6 +73,7 @@ where
     pub fn new() -> Self {
         Self {
             view: EditView::new().content("0"),
+            min_value: None,
             max_value: None,
             max_content_width: None,
             allow_empty: false,
@@ -81,12 +83,20 @@ where
     pub fn new_empty() -> Self {
         Self {
             view: EditView::new(),
+            min_value: None,
             max_value: None,
             max_content_width: None,
             allow_empty: true,
         }
     }
 
+    pub fn with_range(min: T, max: T) -> Self {
+        let mut view = Self::new();
+        view.min_value = Some(min);
+        view.max_value = Some(max);
+        view
+    }
+
     pub fn max_value(mut self, max: T) -> Self {
         self.max_value = Some(max);
         self
@@ -113,12 +123,19 @@ where
         }
     }
 
+    pub fn set_min_value(&mut self, min: T) {
+        self.min_value = Some(min);
+    }
+
     pub fn set_max_value(&mut self, max: T) {
         self.max_value = Some(max);
     }
 
     fn in_range(&self, value: T) -> bool {
-        !self.max_value.map_or(false, |max| value >= max)
+        let too_small = self.min_value.map_or(false, |min| value < min);
+        let too_large = self.max_value.map_or(false, |max| value > max);
+
+        !too_small && !too_large
     }
 
     fn check_bounds(&mut self, original: Rc<String>, result: EventResult) -> EventResult {
@@ -226,6 +243,10 @@ impl DiskSizeEditView {
         Self { view }
     }
 
+    pub fn with_range(min: f64, max: f64) -> Self {
+        Self::new().min_value(min).max_value(max)
+    }
+
     pub fn content(mut self, content: f64) -> Self {
         if let Some(view) = self
             .view
@@ -255,13 +276,17 @@ impl DiskSizeEditView {
         }
     }
 
+    pub fn min_value(mut self, min: f64) -> Self {
+        if let Some(view) = self.get_inner_mut() {
+            view.set_min_value(min);
+        }
+
+        self
+    }
+
     pub fn max_value(mut self, max: f64) -> Self {
-        if let Some(view) = self
-            .view
-            .get_child_mut(0)
-            .and_then(|v| v.downcast_mut::<ResizedView<FloatEditView>>())
-        {
-            view.get_inner_mut().set_max_value(max);
+        if let Some(view) = self.get_inner_mut() {
+            view.set_max_value(max);
         }
 
         self
@@ -281,6 +306,13 @@ impl DiskSizeEditView {
             None => Err(NumericEditViewError::InvalidView),
         }
     }
+
+    fn get_inner_mut(&mut self) -> Option<&mut FloatEditView> {
+        self.view
+            .get_child_mut(0)
+            .and_then(|v| v.downcast_mut::<ResizedView<FloatEditView>>())
+            .map(|v| v.get_inner_mut())
+    }
 }
 
 impl ViewWrapper for DiskSizeEditView {
-- 
2.42.0

[pve-devel] [PATCH installer 4/7] tui: add min/max constraints for ZFS bootdisk parameters

[Christoph Heiss]

Signed-off-by: Christoph Heiss <c.heiss@proxmox.com>
---
 proxmox-tui-installer/src/views/bootdisk.rs | 19 ++++++++++++++++---
 1 file changed, 16 insertions(+), 3 deletions(-)

diff --git a/proxmox-tui-installer/src/views/bootdisk.rs b/proxmox-tui-installer/src/views/bootdisk.rs
index 46bdd9f..8b5b5d2 100644
--- a/proxmox-tui-installer/src/views/bootdisk.rs
+++ b/proxmox-tui-installer/src/views/bootdisk.rs
@@ -20,6 +20,9 @@ use crate::{
 };
 use crate::{setup::ProxmoxProduct, InstallerState};
 
+// See Proxmox::Sys::Block::partition_bootable_disk()
+const MINIMUM_DISK_SIZE: f64 = 2.;
+
 pub struct BootdiskOptionsView {
     view: LinearLayout,
     advanced_options: Rc<RefCell<BootdiskOptions>>,
@@ -501,7 +504,10 @@ impl ZfsBootdiskOptionsView {
     // TODO: Re-apply previous disk selection from `options` correctly
     fn new(disks: &[Disk], options: &ZfsBootdiskOptions) -> Self {
         let inner = FormView::new()
-            .child("ashift", IntegerEditView::new().content(options.ashift))
+            .child(
+                "ashift",
+                IntegerEditView::with_range(9, 13).content(options.ashift),
+            )
             .child(
                 "compress",
                 SelectView::new()
@@ -526,8 +532,15 @@ impl ZfsBootdiskOptionsView {
                             .unwrap_or_default(),
                     ),
             )
-            .child("copies", IntegerEditView::new().content(options.copies))
-            .child("hdsize", DiskSizeEditView::new().content(options.disk_size));
+            .child(
+                "copies",
+                IntegerEditView::with_range(1, 3).content(options.copies),
+            )
+            .child(
+                "hdsize",
+                DiskSizeEditView::with_range(MINIMUM_DISK_SIZE, options.disk_size)
+                    .content(options.disk_size),
+            );
 
         let view = MultiDiskOptionsView::new(disks, &options.selected_disks, inner)
             .top_panel(TextView::new(
-- 
2.42.0

[pve-devel] [PATCH installer 5/7] tui: add min/max contraints for LVM bootdisk parameters

[Christoph Heiss]

Signed-off-by: Christoph Heiss <c.heiss@proxmox.com>
---
 proxmox-tui-installer/src/views/bootdisk.rs | 21 ++++++++++++++-------
 1 file changed, 14 insertions(+), 7 deletions(-)

diff --git a/proxmox-tui-installer/src/views/bootdisk.rs b/proxmox-tui-installer/src/views/bootdisk.rs
index 8b5b5d2..bb421a1 100644
--- a/proxmox-tui-installer/src/views/bootdisk.rs
+++ b/proxmox-tui-installer/src/views/bootdisk.rs
@@ -248,27 +248,34 @@ impl LvmBootdiskOptionsView {
         let view = FormView::new()
             .child(
                 "Total size",
-                DiskSizeEditView::new()
-                    .content(options.total_size)
-                    .max_value(options.total_size),
+                DiskSizeEditView::with_range(MINIMUM_DISK_SIZE, options.total_size)
+                    .content(options.total_size),
             )
             .child(
                 "Swap size",
-                DiskSizeEditView::new_emptyable().content_maybe(options.swap_size),
+                DiskSizeEditView::new_emptyable()
+                    .content_maybe(options.swap_size)
+                    .max_value(options.total_size),
             )
             .child_conditional(
                 is_pve,
                 "Maximum root volume size",
-                DiskSizeEditView::new_emptyable().content_maybe(options.max_root_size),
+                DiskSizeEditView::new_emptyable()
+                    .content_maybe(options.max_root_size)
+                    .max_value(options.total_size),
             )
             .child_conditional(
                 is_pve,
                 "Maximum data volume size",
-                DiskSizeEditView::new_emptyable().content_maybe(options.max_data_size),
+                DiskSizeEditView::new_emptyable()
+                    .content_maybe(options.max_data_size)
+                    .max_value(options.total_size),
             )
             .child(
                 "Minimum free LVM space",
-                DiskSizeEditView::new_emptyable().content_maybe(options.min_lvm_free),
+                DiskSizeEditView::new_emptyable()
+                    .content_maybe(options.min_lvm_free)
+                    .max_value(options.total_size),
             );
 
         Self { view }
-- 
2.42.0

[pve-devel] [PATCH installer 6/7] tui: add min/max contraints for Btrfs bootdisk parameters

[Christoph Heiss]

Signed-off-by: Christoph Heiss <c.heiss@proxmox.com>
---
 proxmox-tui-installer/src/views/bootdisk.rs | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/proxmox-tui-installer/src/views/bootdisk.rs b/proxmox-tui-installer/src/views/bootdisk.rs
index bb421a1..66d909c 100644
--- a/proxmox-tui-installer/src/views/bootdisk.rs
+++ b/proxmox-tui-installer/src/views/bootdisk.rs
@@ -470,7 +470,11 @@ impl BtrfsBootdiskOptionsView {
         let view = MultiDiskOptionsView::new(
             disks,
             &options.selected_disks,
-            FormView::new().child("hdsize", DiskSizeEditView::new().content(options.disk_size)),
+            FormView::new().child(
+                "hdsize",
+                DiskSizeEditView::with_range(MINIMUM_DISK_SIZE, options.disk_size)
+                    .content(options.disk_size),
+            ),
         )
         .top_panel(TextView::new("Btrfs integration is a technology preview!").center());
 
-- 
2.42.0

[pve-devel] [PATCH installer 7/7] tui: views: add some TUI component tests

[Christoph Heiss]

Signed-off-by: Christoph Heiss <c.heiss@proxmox.com>
---
 proxmox-tui-installer/src/main.rs           |  35 ++++++
 proxmox-tui-installer/src/options.rs        |   2 +-
 proxmox-tui-installer/src/views/mod.rs      | 127 +++++++++++++++++++-
 proxmox-tui-installer/src/views/timezone.rs | 109 +++++++++++++++++
 4 files changed, 267 insertions(+), 6 deletions(-)

diff --git a/proxmox-tui-installer/src/main.rs b/proxmox-tui-installer/src/main.rs
index ab990a8..4971071 100644
--- a/proxmox-tui-installer/src/main.rs
+++ b/proxmox-tui-installer/src/main.rs
@@ -937,3 +937,38 @@ impl FromStr for UiMessage {
         }
     }
 }
+
+#[cfg(test)]
+mod tests {
+    pub mod utils {
+        use cursive::{
+            event::{Event, Key},
+            reexports::crossbeam_channel::Sender,
+            view::Nameable,
+            Cursive, CursiveRunner, Vec2, View,
+        };
+
+        pub fn puppet_siv(view: impl View) -> (CursiveRunner<Cursive>, Sender<Option<Event>>) {
+            let backend = cursive::backends::puppet::Backend::init(Some(Vec2::new(80, 24)));
+            let input = backend.input();
+            let mut siv = Cursive::new().into_runner(backend);
+
+            siv.add_layer(view.with_name("root"));
+            input.send(Some(Event::Refresh)).unwrap();
+            siv.step();
+
+            (siv, input)
+        }
+
+        pub fn send_keys(
+            siv: &mut CursiveRunner<Cursive>,
+            input: &Sender<Option<Event>>,
+            keys: Vec<Key>,
+        ) {
+            for key in keys {
+                input.send(Some(Event::Key(key))).unwrap()
+            }
+            siv.step();
+        }
+    }
+}
diff --git a/proxmox-tui-installer/src/options.rs b/proxmox-tui-installer/src/options.rs
index a0a81c9..d3f213d 100644
--- a/proxmox-tui-installer/src/options.rs
+++ b/proxmox-tui-installer/src/options.rs
@@ -275,7 +275,7 @@ impl BootdiskOptions {
     }
 }
 
-#[derive(Clone, Debug)]
+#[derive(Clone, Debug, PartialEq, Eq)]
 pub struct TimezoneOptions {
     pub country: String,
     pub timezone: String,
diff --git a/proxmox-tui-installer/src/views/mod.rs b/proxmox-tui-installer/src/views/mod.rs
index 0fe715e..b95bc20 100644
--- a/proxmox-tui-installer/src/views/mod.rs
+++ b/proxmox-tui-installer/src/views/mod.rs
@@ -18,9 +18,11 @@ pub use table_view::*;
 mod timezone;
 pub use timezone::*;
 
+#[derive(Debug, PartialEq)]
 pub enum NumericEditViewError<T>
 where
-    T: FromStr,
+    T: FromStr + fmt::Debug + PartialEq,
+    <T as FromStr>::Err: fmt::Debug + PartialEq,
 {
     ParseError(<T as FromStr>::Err),
     OutOfRange {
@@ -34,8 +36,8 @@ where
 
 impl<T> fmt::Display for NumericEditViewError<T>
 where
-    T: fmt::Display + FromStr,
-    <T as FromStr>::Err: fmt::Display,
+    T: fmt::Debug + fmt::Display + FromStr + PartialEq,
+    <T as FromStr>::Err: fmt::Debug + fmt::Display + PartialEq,
 {
     fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
         use NumericEditViewError::*;
@@ -68,7 +70,8 @@ pub struct NumericEditView<T> {
 
 impl<T> NumericEditView<T>
 where
-    T: Copy + ToString + FromStr + PartialOrd,
+    T: fmt::Debug + Copy + ToString + FromStr + PartialOrd + PartialEq,
+    <T as FromStr>::Err: fmt::Debug + PartialEq,
 {
     pub fn new() -> Self {
         Self {
@@ -347,7 +350,8 @@ impl<T: 'static + Clone> FormViewGetValue<T> for SelectView<T> {
 
 impl<T> FormViewGetValue<T> for NumericEditView<T>
 where
-    T: Copy + ToString + FromStr + PartialOrd,
+    T: fmt::Debug + Copy + ToString + FromStr + PartialOrd + PartialEq,
+    <T as FromStr>::Err: fmt::Debug + PartialEq,
     NumericEditView<T>: ViewWrapper,
 {
     type Error = NumericEditViewError<T>;
@@ -405,6 +409,7 @@ impl FormViewGetValue<f64> for DiskSizeEditView {
     }
 }
 
+#[derive(PartialEq, Eq)]
 pub enum FormViewError<T> {
     ChildNotFound(usize),
     ValueError(T),
@@ -635,3 +640,115 @@ impl CidrAddressEditView {
 impl ViewWrapper for CidrAddressEditView {
     cursive::wrap_impl!(self.view: LinearLayout);
 }
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+
+    #[test]
+    fn numeric_edit_view_setting_content_preserves_settings() {
+        let mut view = NumericEditView::with_range(2., 4.).content(2.);
+        assert_eq!(view.min_value, Some(2.));
+        assert_eq!(view.max_value, Some(4.));
+        assert_eq!(view.get_content(), Ok(2.));
+
+        view = view.content(4.);
+        assert_eq!(view.get_content(), Ok(4.));
+
+        view = view.content(3.);
+        assert_eq!(view.get_content(), Ok(3.));
+
+        view = view.content(0.);
+        let err = view.get_content();
+        match err {
+            Err(NumericEditViewError::OutOfRange { value, min, max }) => {
+                assert_eq!(value, 0.);
+                assert_eq!(min, Some(2.));
+                assert_eq!(max, Some(4.));
+            }
+            _ => panic!(),
+        }
+        assert_eq!(
+            err.unwrap_err().to_string(),
+            "out of range: 0, must be at least 2 and at most 4".to_owned(),
+        );
+    }
+
+    #[test]
+    fn disk_size_edit_view_setting_content_preserves_settings() {
+        let mut view = DiskSizeEditView::with_range(2., 4.).content(2.);
+        assert_eq!(view.get_inner_mut().unwrap().min_value, Some(2.));
+        assert_eq!(view.get_inner_mut().unwrap().max_value, Some(4.));
+        assert_eq!(view.get_content(), Ok(2.));
+
+        view = view.content(4.);
+        assert_eq!(view.get_content(), Ok(4.));
+
+        view = view.content(3.);
+        assert_eq!(view.get_content(), Ok(3.));
+
+        view = view.content(0.);
+        let err = view.get_content();
+        match err {
+            Err(NumericEditViewError::OutOfRange { value, min, max }) => {
+                assert_eq!(value, 0.);
+                assert_eq!(min, Some(2.));
+                assert_eq!(max, Some(4.));
+            }
+            _ => panic!(),
+        }
+        assert_eq!(
+            err.unwrap_err().to_string(),
+            "out of range: 0, must be at least 2 and at most 4".to_owned()
+        );
+    }
+
+    #[test]
+    fn numeric_edit_view_get_content() {
+        let mut view = NumericEditView::<usize>::new();
+
+        assert_eq!(view.get_content(), Ok(0));
+        view.set_min_value(2);
+        assert!(view.get_content().is_err());
+        view.set_max_value(4);
+        view = view.content(3);
+        assert_eq!(view.get_content(), Ok(3));
+        view = view.content(5);
+        assert!(view.get_content().is_err());
+
+        view.view.set_content("");
+        view.allow_empty = true;
+        assert_eq!(view.get_content(), Err(NumericEditViewError::Empty));
+    }
+
+    #[test]
+    fn form_view_get_value() {
+        let mut view = FormView::new()
+            .child("foo", EditView::new().content("foo"))
+            .child_conditional(false, "fake-bar", EditView::new().content("fake-bar"))
+            .child_conditional(true, "bar", EditView::new().content("bar"))
+            .child("numeric", NumericEditView::with_range(2, 4).content(2))
+            .child("disksize", DiskSizeEditView::with_range(2., 4.).content(2.))
+            .child(
+                "disksize-opt",
+                DiskSizeEditView::new_emptyable()
+                    .max_value(4.)
+                    .content_maybe(Some(2.)),
+            );
+
+        assert_eq!(view.len(), 5);
+        assert_eq!(view.get_value::<EditView, _>(0), Ok("foo".to_string()));
+        assert_eq!(view.get_value::<EditView, _>(1), Ok("bar".to_string()));
+        assert_eq!(view.get_value::<NumericEditView<usize>, _>(2), Ok(2));
+        assert_eq!(view.get_value::<DiskSizeEditView, _>(3), Ok(2.));
+        assert_eq!(view.get_opt_value::<DiskSizeEditView, _>(4), Ok(Some(2.)));
+
+        view.replace_child(
+            4,
+            DiskSizeEditView::new_emptyable()
+                .max_value(4.)
+                .content_maybe(None),
+        );
+        assert_eq!(view.get_opt_value::<DiskSizeEditView, _>(4), Ok(None));
+    }
+}
diff --git a/proxmox-tui-installer/src/views/timezone.rs b/proxmox-tui-installer/src/views/timezone.rs
index bd38a92..88055d0 100644
--- a/proxmox-tui-installer/src/views/timezone.rs
+++ b/proxmox-tui-installer/src/views/timezone.rs
@@ -132,3 +132,112 @@ impl TimezoneOptionsView {
 impl ViewWrapper for TimezoneOptionsView {
     cursive::wrap_impl!(self.view: FormView);
 }
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+    use crate::{setup::CountryInfo, tests::utils::*};
+    use cursive::event::Key;
+    use std::collections::HashMap;
+
+    fn locale_info() -> LocaleInfo {
+        let mut cczones = HashMap::new();
+        let mut countries = HashMap::new();
+        let mut kmap = HashMap::new();
+
+        cczones.insert("at".to_owned(), vec!["Europe/Vienna".to_owned()]);
+        cczones.insert("jp".to_owned(), vec!["Asia/Tokyo".to_owned()]);
+
+        countries.insert(
+            "at".to_owned(),
+            CountryInfo {
+                name: "Austria".to_owned(),
+                zone: "Europe/Vienna".to_owned(),
+                kmap: "de".to_owned(),
+            },
+        );
+        countries.insert(
+            "jp".to_owned(),
+            CountryInfo {
+                name: "Japan".to_owned(),
+                zone: "Asia/Tokyo".to_owned(),
+                kmap: "jp".to_owned(),
+            },
+        );
+
+        kmap.insert(
+            "de".to_owned(),
+            KeyboardMapping {
+                name: "German".to_owned(),
+                id: "de".to_owned(),
+                xkb_layout: "de".to_owned(),
+                xkb_variant: "nodeadkeys".to_owned(),
+            },
+        );
+
+        LocaleInfo {
+            cczones,
+            countries,
+            kmap,
+        }
+    }
+
+    #[test]
+    fn timezone_view_sets_zones_correctly() {
+        let options = TimezoneOptions {
+            country: "at".to_owned(),
+            timezone: "Europe/Vienna".to_owned(),
+            kb_layout: "de".to_owned(),
+        };
+
+        let view = TimezoneOptionsView::new(&locale_info(), &options);
+        let (mut siv, input) = puppet_siv(view);
+
+        assert_eq!(
+            siv.find_name::<TimezoneOptionsView>("root")
+                .unwrap()
+                .get_values(),
+            Ok(TimezoneOptions {
+                country: "at".to_owned(),
+                timezone: "Europe/Vienna".to_owned(),
+                kb_layout: "de".to_owned(),
+            })
+        );
+
+        // Navigate to timezone and select the second element, aka. UTC
+        send_keys(
+            &mut siv,
+            &input,
+            vec![Key::Down, Key::Enter, Key::Down, Key::Enter],
+        );
+
+        assert_eq!(
+            siv.find_name::<TimezoneOptionsView>("root")
+                .unwrap()
+                .get_values(),
+            Ok(TimezoneOptions {
+                country: "at".to_owned(),
+                timezone: "UTC".to_owned(),
+                kb_layout: "de".to_owned(),
+            })
+        );
+
+        // Navigate up again to country and select the second one
+        send_keys(
+            &mut siv,
+            &input,
+            vec![Key::Up, Key::Enter, Key::Down, Key::Enter],
+        );
+
+        assert_eq!(
+            siv.find_name::<TimezoneOptionsView>("root")
+                .unwrap()
+                .get_values(),
+            Ok(TimezoneOptions {
+                country: "jp".to_owned(),
+                timezone: "Asia/Tokyo".to_owned(),
+                kb_layout: "de".to_owned(),
+            })
+        );
+    }
+}
-- 
2.42.0