Merge pull request #101361 from njuptlzf/label-test

parseOperator error message is inconsistent with the behavior
This commit is contained in:
Kubernetes Prow Robot 2021-04-22 23:14:02 -07:00 committed by GitHub
commit 727ecc6353
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
2 changed files with 36 additions and 4 deletions

View File

@ -31,12 +31,15 @@ import (
) )
var ( var (
validRequirementOperators = []string{ unaryOperators = []string{
string(selection.Exists), string(selection.DoesNotExist),
}
binaryOperators = []string{
string(selection.In), string(selection.NotIn), string(selection.In), string(selection.NotIn),
string(selection.Equals), string(selection.DoubleEquals), string(selection.NotEquals), string(selection.Equals), string(selection.DoubleEquals), string(selection.NotEquals),
string(selection.Exists), string(selection.DoesNotExist),
string(selection.GreaterThan), string(selection.LessThan), string(selection.GreaterThan), string(selection.LessThan),
} }
validRequirementOperators = append(binaryOperators, unaryOperators...)
) )
// Requirements is AND of all requirements. // Requirements is AND of all requirements.
@ -140,7 +143,7 @@ type Requirement struct {
// NewRequirement is the constructor for a Requirement. // NewRequirement is the constructor for a Requirement.
// If any of these rules is violated, an error is returned: // If any of these rules is violated, an error is returned:
// (1) The operator can only be In, NotIn, Equals, DoubleEquals, NotEquals, Exists, or DoesNotExist. // (1) The operator can only be In, NotIn, Equals, DoubleEquals, Gt, Lt, NotEquals, Exists, or DoesNotExist.
// (2) If the operator is In or NotIn, the values set must be non-empty. // (2) If the operator is In or NotIn, the values set must be non-empty.
// (3) If the operator is Equals, DoubleEquals, or NotEquals, the values set must contain one value. // (3) If the operator is Equals, DoubleEquals, or NotEquals, the values set must contain one value.
// (4) If the operator is Exists or DoesNotExist, the value set must be empty. // (4) If the operator is Exists or DoesNotExist, the value set must be empty.
@ -753,7 +756,7 @@ func (p *Parser) parseOperator() (op selection.Operator, err error) {
case NotEqualsToken: case NotEqualsToken:
op = selection.NotEquals op = selection.NotEquals
default: default:
return "", fmt.Errorf("found '%s', expected: '=', '!=', '==', 'in', notin'", lit) return "", fmt.Errorf("found '%s', expected: %v", lit, strings.Join(binaryOperators, ", "))
} }
return op, nil return op, nil
} }

View File

@ -17,6 +17,7 @@ limitations under the License.
package labels package labels
import ( import (
"fmt"
"reflect" "reflect"
"strings" "strings"
"testing" "testing"
@ -148,6 +149,7 @@ func expectMatchDirect(t *testing.T, selector, ls Set) {
} }
//lint:ignore U1000 currently commented out in TODO of TestSetMatches //lint:ignore U1000 currently commented out in TODO of TestSetMatches
//nolint:unused,deadcode
func expectNoMatchDirect(t *testing.T, selector, ls Set) { func expectNoMatchDirect(t *testing.T, selector, ls Set) {
if SelectorFromSet(selector).Matches(ls) { if SelectorFromSet(selector).Matches(ls) {
t.Errorf("Wanted '%s' to not match '%s', but it did.", selector, ls) t.Errorf("Wanted '%s' to not match '%s', but it did.", selector, ls)
@ -304,6 +306,33 @@ func TestParserLookahead(t *testing.T) {
} }
} }
func TestParseOperator(t *testing.T) {
testcases := []struct {
token string
expectedError error
}{
{"in", nil},
{"=", nil},
{"==", nil},
{">", nil},
{"<", nil},
{"notin", nil},
{"!=", nil},
{"!", fmt.Errorf("found '%s', expected: %v", selection.DoesNotExist, strings.Join(binaryOperators, ", "))},
{"exists", fmt.Errorf("found '%s', expected: %v", selection.Exists, strings.Join(binaryOperators, ", "))},
{"(", fmt.Errorf("found '%s', expected: %v", "(", strings.Join(binaryOperators, ", "))},
}
for _, testcase := range testcases {
p := &Parser{l: &Lexer{s: testcase.token, pos: 0}, position: 0}
p.scan()
_, err := p.parseOperator()
if ok := reflect.DeepEqual(testcase.expectedError, err); !ok {
t.Errorf("\nexpect err [%v], \nactual err [%v]", testcase.expectedError, err)
}
}
}
func TestRequirementConstructor(t *testing.T) { func TestRequirementConstructor(t *testing.T) {
requirementConstructorTests := []struct { requirementConstructorTests := []struct {
Key string Key string