Patchwork [2,of,2,V4] revset: raise ValueError when calling min or max on empty smartset

login
register
mail settings
Submitter Pierre-Yves David
Date March 31, 2014, 7:44 p.m.
Message ID <df92bd33c9002bdfcc3d.1396295072@marginatus.alto.octopoid.net>
Download mbox | patch
Permalink /patch/4163/
State Accepted
Commit 876c17336b4ef15559d5cb14dccf73e48f854d77
Headers show

Comments

Pierre-Yves David - March 31, 2014, 7:44 p.m.
# HG changeset patch
# User Pierre-Yves David <pierre-yves.david@fb.com>
# Date 1396051213 25200
#      Fri Mar 28 17:00:13 2014 -0700
# Node ID df92bd33c9002bdfcc3d5faace9ea2860d612324
# Parent  7fc172e2b564a07bfb3ea386d9805967846c2a02
revset: raise ValueError when calling min or max on empty smartset

min([]) raise a ValueError, we do the same thing in smartset.min() and
smartset.max() for the sake of consistency.

The min/amax test are greatly improved in the process to prevent this familly
of regression
David Soria Parra - March 31, 2014, 8:59 p.m.
pierre-yves.david@ens-lyon.org writes:

> # HG changeset patch
> # User Pierre-Yves David <pierre-yves.david@fb.com>
> # Date 1396051213 25200
> #      Fri Mar 28 17:00:13 2014 -0700
> # Node ID df92bd33c9002bdfcc3d5faace9ea2860d612324
> # Parent  7fc172e2b564a07bfb3ea386d9805967846c2a02
> revset: raise ValueError when calling min or max on empty smartset
>
> min([]) raise a ValueError, we do the same thing in smartset.min() and
> smartset.max() for the sake of consistency.
>
> The min/amax test are greatly improved in the process to prevent this familly
> of regression
>

Queued for default. Thank you.

Patch

diff --git a/mercurial/revset.py b/mercurial/revset.py
--- a/mercurial/revset.py
+++ b/mercurial/revset.py
@@ -2285,11 +2285,11 @@  class _orderedsetmixin(object):
 
     def _first(self):
         """return the first revision in the set"""
         for r in self:
             return r
-        return None
+        raise ValueError('arg is an empty sequence')
 
     def _last(self):
         """return the last revision in the set"""
         self.reverse()
         m = self._first()
diff --git a/tests/test-revset.t b/tests/test-revset.t
--- a/tests/test-revset.t
+++ b/tests/test-revset.t
@@ -353,14 +353,50 @@  ancestor can accept 0 or more arguments
   $ log 'matching(6)'
   6
   $ log 'matching(6:7, "phase parents user date branch summary files description substate")'
   6
   7
+
+Testing min and max
+
+max: simple
+
   $ log 'max(contains(a))'
   5
+
+max: simple on unordered set)
+
+  $ log 'max((4+0+2+5+7) and contains(a))'
+  5
+
+max: no result
+
+  $ log 'max(contains(stringthatdoesnotappearanywhere))'
+
+max: no result on unordered set
+
+  $ log 'max((4+0+2+5+7) and contains(stringthatdoesnotappearanywhere))'
+
+min: simple
+
   $ log 'min(contains(a))'
   0
+
+min: simple on unordered set
+
+  $ log 'min((4+0+2+5+7) and contains(a))'
+  0
+
+min: empty
+
+  $ log 'min(contains(stringthatdoesnotappearanywhere))'
+
+min: empty on unordered set
+
+  $ log 'min((4+0+2+5+7) and contains(stringthatdoesnotappearanywhere))'
+
+
   $ log 'merge()'
   6
   $ log 'branchpoint()'
   1
   4