From 4c89a9c33c4c097836f5bfa79cc7e5adc142a2f0 Mon Sep 17 00:00:00 2001 From: Ethan Koenig Date: Thu, 22 Dec 2016 03:58:04 -0500 Subject: [PATCH] Bug fixes and tests for modules/base (#442) Also address other TODOs --- models/issue.go | 14 ++- modules/base/tool.go | 45 +++++--- modules/base/tool_test.go | 233 +++++++++++++++++++++++++++++++++++--- routers/repo/issue.go | 5 +- 4 files changed, 261 insertions(+), 36 deletions(-) diff --git a/models/issue.go b/models/issue.go index 8d8c304666..0e6c794055 100644 --- a/models/issue.go +++ b/models/issue.go @@ -915,7 +915,10 @@ func Issues(opts *IssuesOptions) ([]*Issue, error) { } if len(opts.Labels) > 0 && opts.Labels != "0" { - labelIDs := base.StringsToInt64s(strings.Split(opts.Labels, ",")) + labelIDs, err := base.StringsToInt64s(strings.Split(opts.Labels, ",")) + if err != nil { + return nil, err + } if len(labelIDs) > 0 { sess. Join("INNER", "issue_label", "issue.id = issue_label.issue_id"). @@ -1171,10 +1174,11 @@ func GetIssueStats(opts *IssueStatsOptions) *IssueStats { And("is_pull = ?", opts.IsPull) if len(opts.Labels) > 0 && opts.Labels != "0" { - labelIDs := base.StringsToInt64s(strings.Split(opts.Labels, ",")) - if len(labelIDs) > 0 { - sess. - Join("INNER", "issue_label", "issue.id = issue_id"). + labelIDs, err := base.StringsToInt64s(strings.Split(opts.Labels, ",")) + if err != nil { + log.Warn("Malformed Labels argument: %s", opts.Labels) + } else if len(labelIDs) > 0 { + sess.Join("INNER", "issue_label", "issue.id = issue_id"). In("label_id", labelIDs) } } diff --git a/modules/base/tool.go b/modules/base/tool.go index eb25108869..cb2eafd309 100644 --- a/modules/base/tool.go +++ b/modules/base/tool.go @@ -56,6 +56,9 @@ func DetectEncoding(content []byte) (string, error) { } result, err := chardet.NewTextDetector().DetectBest(content) + if err != nil { + return "", err + } if result.Charset != "UTF-8" && len(setting.Repository.AnsiCharset) > 0 { log.Debug("Using default AnsiCharset: %s", setting.Repository.AnsiCharset) return setting.Repository.AnsiCharset, err @@ -256,19 +259,25 @@ func computeTimeDiff(diff int64) (int64, string) { diffStr = "1 year" default: diffStr = fmt.Sprintf("%d years", diff/Year) - diff = 0 + diff -= (diff / Year) * Year } return diff, diffStr } // TimeSincePro calculates the time interval and generate full user-friendly string. func TimeSincePro(then time.Time) string { - now := time.Now() + return timeSincePro(then, time.Now()) +} + +func timeSincePro(then, now time.Time) string { diff := now.Unix() - then.Unix() if then.After(now) { return "future" } + if diff == 0 { + return "now" + } var timeStr, diffStr string for { @@ -282,9 +291,7 @@ func TimeSincePro(then time.Time) string { return strings.TrimPrefix(timeStr, ", ") } -func timeSince(then time.Time, lang string) string { - now := time.Now() - +func timeSince(then, now time.Time, lang string) string { lbl := i18n.Tr(lang, "tool.ago") diff := now.Unix() - then.Unix() if then.After(now) { @@ -295,7 +302,7 @@ func timeSince(then time.Time, lang string) string { switch { case diff <= 0: return i18n.Tr(lang, "tool.now") - case diff <= 2: + case diff <= 1: return i18n.Tr(lang, "tool.1s", lbl) case diff < 1*Minute: return i18n.Tr(lang, "tool.seconds", diff, lbl) @@ -334,12 +341,18 @@ func timeSince(then time.Time, lang string) string { // RawTimeSince retrieves i18n key of time since t func RawTimeSince(t time.Time, lang string) string { - return timeSince(t, lang) + return timeSince(t, time.Now(), lang) } // TimeSince calculates the time interval and generate user-friendly string. -func TimeSince(t time.Time, lang string) template.HTML { - return template.HTML(fmt.Sprintf(`%s`, t.Format(setting.TimeFormat), timeSince(t, lang))) +func TimeSince(then time.Time, lang string) template.HTML { + return htmlTimeSince(then, time.Now(), lang) +} + +func htmlTimeSince(then, now time.Time, lang string) template.HTML { + return template.HTML(fmt.Sprintf(`%s`, + then.Format(setting.TimeFormat), + timeSince(then, now, lang))) } // Storage space size types @@ -424,10 +437,10 @@ func Subtract(left interface{}, right interface{}) interface{} { case int64: rright = right.(int64) case float32: - fright = float64(left.(float32)) + fright = float64(right.(float32)) isInt = false case float64: - fleft = left.(float64) + fright = right.(float64) isInt = false } @@ -459,12 +472,16 @@ func TruncateString(str string, limit int) string { } // StringsToInt64s converts a slice of string to a slice of int64. -func StringsToInt64s(strs []string) []int64 { +func StringsToInt64s(strs []string) ([]int64, error) { ints := make([]int64, len(strs)) for i := range strs { - ints[i] = com.StrTo(strs[i]).MustInt64() + n, err := com.StrTo(strs[i]).Int64() + if err != nil { + return ints, err + } + ints[i] = n } - return ints + return ints, nil } // Int64sToStrings converts a slice of int64 to a slice of string. diff --git a/modules/base/tool_test.go b/modules/base/tool_test.go index 2ca70b8b32..9c4fb89e8e 100644 --- a/modules/base/tool_test.go +++ b/modules/base/tool_test.go @@ -4,10 +4,40 @@ import ( "testing" "code.gitea.io/gitea/modules/setting" + "github.com/Unknwon/i18n" + macaroni18n "github.com/go-macaron/i18n" "github.com/stretchr/testify/assert" + "os" "strk.kbt.io/projects/go/libravatar" + "time" ) +var BaseDate time.Time + +// time durations +const ( + DayDur = 24 * time.Hour + WeekDur = 7 * DayDur + MonthDur = 30 * DayDur + YearDur = 12 * MonthDur +) + +func TestMain(m *testing.M) { + // setup + macaroni18n.I18n(macaroni18n.Options{ + Directory: "../../conf/locale/", + DefaultLang: "en-US", + Langs: []string{"en-US"}, + Names: []string{"english"}, + }) + BaseDate = time.Date(2000, time.January, 1, 0, 0, 0, 0, time.UTC) + + // run the tests + retVal := m.Run() + + os.Exit(retVal) +} + func TestEncodeMD5(t *testing.T) { assert.Equal(t, "3858f62230ac3c915f300c664312c63f", @@ -26,7 +56,36 @@ func TestShortSha(t *testing.T) { assert.Equal(t, "veryverylo", ShortSha("veryverylong")) } -// TODO: Test DetectEncoding() +func TestDetectEncoding(t *testing.T) { + testSuccess := func(b []byte, expected string) { + encoding, err := DetectEncoding(b) + assert.NoError(t, err) + assert.Equal(t, expected, encoding) + } + // utf-8 + b := []byte("just some ascii") + testSuccess(b, "UTF-8") + + // utf-8-sig: "hey" (with BOM) + b = []byte{0xef, 0xbb, 0xbf, 0x68, 0x65, 0x79} + testSuccess(b, "UTF-8") + + // utf-16: "hey" + b = []byte{0xff, 0xfe, 0x68, 0x00, 0x65, 0x00, 0x79, 0x00, 0xf4, 0x01} + testSuccess(b, "UTF-16LE") + + // iso-8859-1: dcor + b = []byte{0x44, 0xe9, 0x63, 0x6f, 0x72, 0x0a} + testSuccess(b, "ISO-8859-1") + + setting.Repository.AnsiCharset = "placeholder" + testSuccess(b, "placeholder") + + // invalid bytes + b = []byte{0xfa} + _, err := DetectEncoding(b) + assert.Error(t, err) +} func TestBasicAuthDecode(t *testing.T) { _, _, err := BasicAuthDecode("?") @@ -88,11 +147,112 @@ func TestAvatarLink(t *testing.T) { ) } -// TODO: computeTimeDiff() -// TODO: TimeSincePro() -// TODO: timeSince() -// TODO: RawTimeSince() -// TODO: TimeSince() +func TestComputeTimeDiff(t *testing.T) { + // test that for each offset in offsets, + // computeTimeDiff(base + offset) == (offset, str) + test := func(base int64, str string, offsets ...int64) { + for _, offset := range offsets { + diff, diffStr := computeTimeDiff(base + offset) + assert.Equal(t, offset, diff) + assert.Equal(t, str, diffStr) + } + } + test(0, "now", 0) + test(1, "1 second", 0) + test(2, "2 seconds", 0) + test(Minute, "1 minute", 0, 1, 30, Minute-1) + test(2*Minute, "2 minutes", 0, Minute-1) + test(Hour, "1 hour", 0, 1, Hour-1) + test(5*Hour, "5 hours", 0, Hour-1) + test(Day, "1 day", 0, 1, Day-1) + test(5*Day, "5 days", 0, Day-1) + test(Week, "1 week", 0, 1, Week-1) + test(3*Week, "3 weeks", 0, 4*Day+25000) + test(Month, "1 month", 0, 1, Month-1) + test(10*Month, "10 months", 0, Month-1) + test(Year, "1 year", 0, Year-1) + test(3*Year, "3 years", 0, Year-1) +} + +func TestTimeSince(t *testing.T) { + assert.Equal(t, "now", timeSince(BaseDate, BaseDate, "en")) + + // test that each diff in `diffs` yields the expected string + test := func(expected string, diffs ...time.Duration) { + ago := i18n.Tr("en", "tool.ago") + fromNow := i18n.Tr("en", "tool.from_now") + for _, diff := range diffs { + actual := timeSince(BaseDate, BaseDate.Add(diff), "en") + assert.Equal(t, expected+" "+ago, actual) + actual = timeSince(BaseDate.Add(diff), BaseDate, "en") + assert.Equal(t, expected+" "+fromNow, actual) + } + } + test("1 second", time.Second, time.Second+50*time.Millisecond) + test("2 seconds", 2*time.Second, 2*time.Second+50*time.Millisecond) + test("1 minute", time.Minute, time.Minute+30*time.Second) + test("2 minutes", 2*time.Minute, 2*time.Minute+30*time.Second) + test("1 hour", time.Hour, time.Hour+30*time.Minute) + test("2 hours", 2*time.Hour, 2*time.Hour+30*time.Minute) + test("1 day", DayDur, DayDur+12*time.Hour) + test("2 days", 2*DayDur, 2*DayDur+12*time.Hour) + test("1 week", WeekDur, WeekDur+3*DayDur) + test("2 weeks", 2*WeekDur, 2*WeekDur+3*DayDur) + test("1 month", MonthDur, MonthDur+15*DayDur) + test("2 months", 2*MonthDur, 2*MonthDur+15*DayDur) + test("1 year", YearDur, YearDur+6*MonthDur) + test("2 years", 2*YearDur, 2*YearDur+6*MonthDur) +} + +func TestTimeSincePro(t *testing.T) { + assert.Equal(t, "now", timeSincePro(BaseDate, BaseDate)) + + // test that a difference of `diff` yields the expected string + test := func(expected string, diff time.Duration) { + actual := timeSincePro(BaseDate, BaseDate.Add(diff)) + assert.Equal(t, expected, actual) + assert.Equal(t, "future", timeSincePro(BaseDate.Add(diff), BaseDate)) + } + test("1 second", time.Second) + test("2 seconds", 2*time.Second) + test("1 minute", time.Minute) + test("1 minute, 1 second", time.Minute+time.Second) + test("1 minute, 59 seconds", time.Minute+59*time.Second) + test("2 minutes", 2*time.Minute) + test("1 hour", time.Hour) + test("1 hour, 1 second", time.Hour+time.Second) + test("1 hour, 59 minutes, 59 seconds", time.Hour+59*time.Minute+59*time.Second) + test("2 hours", 2*time.Hour) + test("1 day", DayDur) + test("1 day, 23 hours, 59 minutes, 59 seconds", + DayDur+23*time.Hour+59*time.Minute+59*time.Second) + test("2 days", 2*DayDur) + test("1 week", WeekDur) + test("2 weeks", 2*WeekDur) + test("1 month", MonthDur) + test("3 months", 3*MonthDur) + test("1 year", YearDur) + test("2 years, 3 months, 1 week, 2 days, 4 hours, 12 minutes, 17 seconds", + 2*YearDur+3*MonthDur+WeekDur+2*DayDur+4*time.Hour+ + 12*time.Minute+17*time.Second) +} + +func TestHtmlTimeSince(t *testing.T) { + setting.TimeFormat = time.UnixDate + // test that `diff` yields a result containing `expected` + test := func(expected string, diff time.Duration) { + actual := htmlTimeSince(BaseDate, BaseDate.Add(diff), "en") + assert.Contains(t, actual, `title="Sat Jan 1 00:00:00 UTC 2000"`) + assert.Contains(t, actual, expected) + } + test("1 second", time.Second) + test("3 minutes", 3*time.Minute+5*time.Second) + test("1 day", DayDur+18*time.Hour) + test("1 week", WeekDur+6*DayDur) + test("3 months", 3*MonthDur+3*WeekDur) + test("2 years", 2*YearDur) + test("3 years", 3*YearDur+11*MonthDur+4*WeekDur) +} func TestFileSize(t *testing.T) { var size int64 @@ -108,11 +268,48 @@ func TestFileSize(t *testing.T) { assert.Equal(t, "512TB", FileSize(size)) size = size * 1024 assert.Equal(t, "512PB", FileSize(size)) - //size = size * 1024 TODO: Fix bug for EB - //assert.Equal(t, "512EB", FileSize(size)) + size = size * 4 + assert.Equal(t, "2.0EB", FileSize(size)) } -// TODO: Subtract() +func TestSubtract(t *testing.T) { + toFloat64 := func(n interface{}) float64 { + switch n.(type) { + case int: + return float64(n.(int)) + case int8: + return float64(n.(int8)) + case int16: + return float64(n.(int16)) + case int32: + return float64(n.(int32)) + case int64: + return float64(n.(int64)) + case float32: + return float64(n.(float32)) + case float64: + return n.(float64) + default: + return 0.0 + } + } + values := []interface{}{ + int(-3), + int8(14), + int16(81), + int32(-156), + int64(1528), + float32(3.5), + float64(-15.348), + } + for _, left := range values { + for _, right := range values { + expected := toFloat64(left) - toFloat64(right) + sub := Subtract(left, right) + assert.InDelta(t, expected, sub, 1e-3) + } + } +} func TestEllipsisString(t *testing.T) { assert.Equal(t, "...", EllipsisString("foobar", 0)) @@ -137,14 +334,18 @@ func TestTruncateString(t *testing.T) { } func TestStringsToInt64s(t *testing.T) { - assert.Equal(t, []int64{}, StringsToInt64s([]string{})) - assert.Equal(t, - []int64{1, 4, 16, 64, 256}, - StringsToInt64s([]string{"1", "4", "16", "64", "256"}), - ) + testSuccess := func(input []string, expected []int64) { + result, err := StringsToInt64s(input) + assert.NoError(t, err) + assert.Equal(t, expected, result) + } + testSuccess([]string{}, []int64{}) + testSuccess([]string{"-1234"}, []int64{-1234}) + testSuccess([]string{"1", "4", "16", "64", "256"}, + []int64{1, 4, 16, 64, 256}) - // TODO: StringsToInt64s should return ([]int64, error) - assert.Equal(t, []int64{-1, 0, 0}, StringsToInt64s([]string{"-1", "a", "$"})) + _, err := StringsToInt64s([]string{"-1", "a", "$"}) + assert.Error(t, err) } func TestInt64sToStrings(t *testing.T) { diff --git a/routers/repo/issue.go b/routers/repo/issue.go index e3c088d940..343b93e273 100644 --- a/routers/repo/issue.go +++ b/routers/repo/issue.go @@ -374,7 +374,10 @@ func ValidateRepoMetas(ctx *context.Context, form auth.CreateIssueForm) ([]int64 } // Check labels. - labelIDs := base.StringsToInt64s(strings.Split(form.LabelIDs, ",")) + labelIDs, err := base.StringsToInt64s(strings.Split(form.LabelIDs, ",")) + if err != nil { + return nil, 0, 0 + } labelIDMark := base.Int64sToMap(labelIDs) hasSelected := false for i := range labels {