commonsearch / cosr-front

Frontend of Common Search. Go server for fetching and rendering results + HTML5 UI to browse them.
https://uidemo.commonsearch.org
Apache License 2.0
60 stars 13 forks source link

Investigate a better Go test framework #8

Open sylvinus opened 8 years ago

sylvinus commented 8 years ago

Being used to the awesome pytest, I have a hard time with Go's testing package.

Are there friendlier alternatives that would help us enjoy writing more tests (which we need)? What about http://labix.org/gocheck ?

sylvinus commented 8 years ago

Adding coverage as part of this issue would be great too! We could send it to coveralls like we do for cosr-back https://coveralls.io/github/commonsearch/cosr-back

ghost commented 8 years ago

@sylvinus I would use the standard testing library, but modify the tests. Now there are too many repetitions. E.g. bangs_test.go:

if DetectBang("", "en") != "" {
    t.Fatal("Empty search")
}

if DetectBang("!a littlebits", "en") != "https://www.amazon.com/s/?field-keywords=littlebits" {
    t.Fatal("Amazon bang EN")
}

if DetectBang("!a littlebits", "fr") != "https://www.amazon.fr/s/?field-keywords=littlebits" {
    t.Fatal("Amazon bang FR")
}

if DetectBang("!w littlebits", "en") != "https://en.wikipedia.org/wiki/Special:Search?search=littlebits" {
    t.Fatal("Wikipedia bang EN (via any)")
}

Instead that could be:

for _, v := range []struct{
    query, lang, exp string
}{
    {"", "en", ""},
    {"!a littlebits", "en", "https://www.amazon.com/s/?field-keywords=littlebits"},
    {"!a littlebits", "fr", "https://www.amazon.fr/s/?field-keywords=littlebits"},
    {"!w littlebits", "en", "https://en.wikipedia.org/wiki/Special:Search?search=littlebits"},
} {
    if res := DetectBang(v.query, v.lang); res != v.exp {
        t.Errorf(`Query "%s" (%s). Expected: "%s", got: "%s".`, v.query, v.lang, v.exp, res)
    }
}

Now you can add new tests without extra boilerplate code and modify existing ones much easier.

sylvinus commented 8 years ago

Thanks @alkchr.

I think it still looks more complicated than it could be. Feels like we're re-implementing a testing package.

With gocheck for instance, the inner loop would only be c.Assert(DetectBang(v.query, v.lang), Equals, v.exp)?

ghost commented 8 years ago

@sylvinus We're not reimplementing anything. Anonymous structs is a frequently used technique.

As for gocheck's Assert function, it reduces the number of LOC required. But it doesn't print the input parameters (query and lang) of the failed test. In case of TestBangs it is not critical, but what if your tests are:

{
    {"...", "en", "english_version"},
    {"...", "unknown_language", "english_version"},
}

In both cases expected output is english_version. If Assert shows Expected "english_version", got "french_version", how do you know which of the tests above failed?

wumpus commented 8 years ago

I'm OK with anonymous structs, that's how most of blekko's unit tests and integration tests were expressed. And yes, clear explanations of what failed is very important.