Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update tests to use ALL command-line flags #187

Open
atc0005 opened this issue Dec 13, 2019 · 1 comment
Open

Update tests to use ALL command-line flags #187

atc0005 opened this issue Dec 13, 2019 · 1 comment
Assignees
Labels
bug Something isn't working tests
Milestone

Comments

@atc0005
Copy link
Owner

atc0005 commented Dec 13, 2019

While working on #186, I encountered this test failure:

--- FAIL: TestValidate (0.00s)
    --- FAIL: TestValidate/LogLevel_set_to_valid_values (0.00s)
        validate_test.go:539: Config failed, but should have passed on valid value "emerg" for LogLevel: invalid option "emerg" provided for log level
        validate_test.go:536: Config passed as expected after setting LogLevel to "alert": <nil>
        validate_test.go:536: Config passed as expected after setting LogLevel to "critical": <nil>
        validate_test.go:536: Config passed as expected after setting LogLevel to "panic": <nil>
        validate_test.go:536: Config passed as expected after setting LogLevel to "fatal": <nil>
        validate_test.go:536: Config passed as expected after setting LogLevel to "error": <nil>
        validate_test.go:536: Config passed as expected after setting LogLevel to "warn": <nil>
        validate_test.go:536: Config passed as expected after setting LogLevel to "info": <nil>
        validate_test.go:536: Config passed as expected after setting LogLevel to "notice": <nil>
        validate_test.go:536: Config passed as expected after setting LogLevel to "debug": <nil>
        validate_test.go:536: Config passed as expected after setting LogLevel to "trace": <nil>
        validate_test.go:549: Validation successful after restoring LogLevel field

I had just replaced the "emergency" string here:

tests := []string{
"emergency",
"alert",
"critical",
"panic",
"fatal",
"error",
"warn",
"info",
"notice",
"debug",
"trace",
}

with the new constant:

                tests := []string{
-                       "emergency",
+                       logging.LogLevelEmerg,

which is defined as:

	// unusable; a panic condition. This level is mapped to logrus.PanicLevel
	// and syslog.LOG_EMERG. logrus calls panic.
	LogLevelEmerg string = "emerg"

I pulled that from here:

switch logLevel {
// TODO: Evaluate replacing bare strings with constants (see constants.go)
case "emerg", "panic":
// syslog: System is unusable; a panic condition.
// logrus: calls panic
syslogLogLevel = syslog.LOG_EMERG

which is being "tested" like so:

tests := []test{
test{logLevel: "emerg", loggerLevel: logrus.PanicLevel},
test{logLevel: "panic", loggerLevel: logrus.PanicLevel},
test{logLevel: "alert", loggerLevel: logrus.FatalLevel},
test{logLevel: "critical", loggerLevel: logrus.FatalLevel},
test{logLevel: "fatal", loggerLevel: logrus.FatalLevel},
test{logLevel: "error", loggerLevel: logrus.ErrorLevel},
test{logLevel: "warn", loggerLevel: logrus.WarnLevel},
test{logLevel: "notice", loggerLevel: logrus.WarnLevel},
test{logLevel: "info", loggerLevel: logrus.InfoLevel},
test{logLevel: "debug", loggerLevel: logrus.DebugLevel},
test{logLevel: "trace", loggerLevel: logrus.TraceLevel},
}

The expected log level value is being checked like so:

case *c.LogLevel == "emergency":

which apparently only checks against the command-line flag value, not what is keyed off of to do the final syslog and logrus calls.

@atc0005 atc0005 added bug Something isn't working tests labels Dec 13, 2019
@atc0005 atc0005 added this to the v0.7.0 milestone Dec 13, 2019
@atc0005 atc0005 self-assigned this Dec 13, 2019
@atc0005
Copy link
Owner Author

atc0005 commented Dec 13, 2019

I checked the README to be sure, and found that the emergency option for ``--log-level` is what we have documented as the valid command-line flag option:

https://github.com/atc0005/elbow/blame/788742bc682e57958d30113e52355efffc8d7b32/README.md#L202

I vaguely recall writing the Validate() function based on that literal value.

@atc0005 atc0005 modified the milestones: v0.7.0, Future Jan 15, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working tests
Projects
None yet
Development

No branches or pull requests

1 participant