[PATCH 6/6] Added a validation test for the GUI group list

Vratislav Podzimek vpodzime at redhat.com
Thu Aug 22 09:27:00 UTC 2013


On Wed, 2013-08-21 at 10:40 -0400, David Shea wrote:
> On 08/21/2013 05:11 AM, Vratislav Podzimek wrote:
> > On Tue, 2013-08-20 at 15:38 -0400, David Shea wrote:
> >> ---
> >>   pyanaconda/constants.py                      | 16 ++++++
> >>   pyanaconda/ui/gui/__init__.py                | 77 ++++++++++++++++++++++++++++
> >>   pyanaconda/ui/gui/spokes/advanced_user.glade | 24 ++++++---
> >>   pyanaconda/ui/gui/spokes/user.py             | 32 ++++++++++--
> >>   tests/regex_tests/groupparse_test.py         | 69 +++++++++++++++++++++++++
> >>   5 files changed, 208 insertions(+), 10 deletions(-)
> >>   create mode 100755 tests/regex_tests/groupparse_test.py
> >>
> >> diff --git a/pyanaconda/constants.py b/pyanaconda/constants.py
> >> index 086ece8..d47590e 100644
> >> --- a/pyanaconda/constants.py
> >> +++ b/pyanaconda/constants.py
> >> @@ -173,5 +173,21 @@ GROUPNAME_VALID = USERNAME_VALID
> >>   # before and after the commas. The empty string is allowed.
> >>   GROUPLIST_SIMPLE_VALID = re.compile(r'^\s*(' + _USERNAME_BASE + r'(\s*,\s*' + _USERNAME_BASE + r')*)?\s*$')
> >>   
> >> +# Parse the <gr-name> (<gid>) strings in the group list.
> >> +#
> >> +# The name match is non-greedy so that it doesn't match the whitespace betweeen
> >> +# the name and ID.
> >> +#
> >> +# There's some non-capturing groups ("clusters" in the perlre parlance) thrown
> >> +# in there, and, haha, wow, that's confusing to look at. There are two groups
> >> +# that actually end up in the match object, and they're named to try to make
> >> +# it a little easier: the first is "name", and the second is "gid".
> >> +#
> >> +# EVERY STRING IS MATCHED. This expression cannot be used for validation.
> >> +# If there is no GID, or the GID contains non-digits, everything except
> >> +# leading or trailing whitespace ends up in the name group. The result needs to
> >> +# be validated with GROUPNAME_VALID.
> >> +GROUPLIST_FANCY_PARSE = re.compile(r'^(?:\s*)(?P<name>.*?)\s*(?:\((?P<gid>\d+)\))?(?:\s*)$')
> >> +
> >>   # Proxy parsing
> >>   PROXY_URL_PARSE = re.compile("([A-Za-z]+://)?(([A-Za-z0-9]+)(:[^:@]+)?@)?([^:/]+)(:[0-9]+)?(/.*)?")
> >> diff --git a/pyanaconda/ui/gui/__init__.py b/pyanaconda/ui/gui/__init__.py
> >> index f408257..6174409 100644
> >> --- a/pyanaconda/ui/gui/__init__.py
> >> +++ b/pyanaconda/ui/gui/__init__.py
> >> @@ -343,6 +343,83 @@ class GUIObject(common.UIObject):
> >>           """All failed input checks"""
> >>           return (c for c in self._check_list if c.check_status)
> >>   
> >> +class GUIDialog(GUIObject):
> >> +    """This is an abstract for creating dialog windows. It implements the
> >> +       update_check interface to display an error message when an input
> >> +       validation fails.
> >> +
> >> +       GUIDialog does not define where errors are displayed, so classes
> >> +       that derive from GUIDialog must define error labels and include them
> >> +       as a parameter to add_check. More than one check can use the same
> >> +       label: the message from the first failed check will update the label.
> >> +    """
> > Do we really need a special class for that? Especially if it overrides
> > parent class' methods in a potentially problematic way? I'd suggest
> > making add_check more general by taking a check function and set_error
> > function, that would make sure the errors is displayed to the user. That
> > would also allow to remove the updateCheck method of the GUIObject.
> I'm ok with passing a set_error function to add_check, but I still think 
> that having a GUIDialog class would be helpful, since displaying an 
> error in a dialog is so different from displaying one in a spoke. Or at 
> least it will be helpful if we ever have more than one dialog window 
> trying to use the add_check stuff. I agree that overriding the parent 
> methods was a bad idea. Maybe require that the check_data parameter be a 
> tuple containing a label, and raise a ValueError if it isn't?
I think that by passing set_error function to the add_check method would
make it more general and better usable. Moreover, a single check would
trigger a single set_error function instead of updating check status for
all checks everytime a single editable with a check assigned is changed.
And you can use the default value of the set_error_fn argument to use
some method/function by default.

The GUIDialog class then may have some method like
"add_check_with_error_label" that would link the check with a function
that would update the label.

-- 
Vratislav Podzimek

Anaconda Rider | Red Hat, Inc. | Brno - Czech Republic



More information about the anaconda-patches mailing list