Start to add tests for modules/base/tool #90
Conversation
Current coverage is 2.47% (diff: 100%)@@ master #90 diff @@
========================================
Files 31 32 +1
Lines 7508 7812 +304
Methods 0 0
Messages 0 0
Branches 0 0
========================================
+ Hits 164 193 +29
- Misses 7327 7601 +274
- Partials 17 18 +1
|
| } | ||
|
|
||
| func TestShortSha(t *testing.T) { | ||
| if result := ShortSha("veryverylong"); result != "veryverylo" { |
There was a problem hiding this comment.
please use a hex-value when testing hash-functions 🙂
There was a problem hiding this comment.
I thought this was a hash-function too. But if you look closer it only shortens a string. Pretty bad function naming. But I don't want to change the API with the tests. That should be a different PR.
There was a problem hiding this comment.
until it's fixed, please use ShaSums ;)
|
|
||
| func TestShortSha(t *testing.T) { | ||
| if result := ShortSha("veryverylong"); result != "veryverylo" { | ||
| t.Errorf("got the wrong sha1sum for string foobar: %s", result) |
| // TODO: Test CreateTimeLimitCode() | ||
|
|
||
| func TestHashEmail(t *testing.T) { | ||
| if hash := HashEmail("[email protected]"); hash != "1b6d0c0e124d47ded12cd7115addeb11" { |
There was a problem hiding this comment.
I'd refrain against using real addresses when not necessary, this is why ’example.com’ exists 🙂
|
Hey @metalmatze, great work on adding tests, i have noticed that you are in some cases repeating and duplicating assert calls (for example |
| setting.LibravatarService = libravatar.New() | ||
| assert.Equal(t, | ||
| "http://cdn.libravatar.org/avatar/353cbad9b58e69c96154ad99f92bedc7", | ||
| AvatarLink("[email protected]"), |
There was a problem hiding this comment.
If you need a test for a really federated avatar, try [email protected]
There was a problem hiding this comment.
Doesn't make a difference for the test. right?
There was a problem hiding this comment.
well, this one isn't really testing the federated part (I've seen packages just prefixing the hash with "cdn.libravatar.org" and be happy with it, it's not our case, we're actually doing the DNS lookups)
|
Hey @jbub. Thanks for the hint. |
metalmatze
changed the title
|
I'm done for now. I think we could merge this and add more tests in another PR. This one is pretty big anyway. Updated all the stuff @bkcsoft said. 👍 LGTY? 😄 |
Current coverage is 3.14% (diff: 100%)@@ master #90 diff @@
========================================
Files 32 33 +1
Lines 7521 7821 +300
Methods 0 0
Messages 0 0
Branches 0 0
========================================
+ Hits 169 246 +77
- Misses 7335 7555 +220
- Partials 17 20 +3
|
|
LGTM |
|
Awesome work, LGTM |
|
LGTM |
tboerger
added
the
lgtm/done
* Add an head ref for the sake of using self repo for testing * Add test for CommitCount * Add testing with git-1.7.2 * Add test for GetLatestCommitTime The test checks that latest commit time is before now and more recent than the commit this PR is based at Test no error is raised by time parsing and GetLatestCommitTime Print actual time when tests fail * Accept git 1.7.2 as the minimum version Debian old old (very old) distribution (6.0 aka Squeeze) ships version 1.7.10.4. The version requirement was raised in go-gitea#46 supposedly for the need of "symbolic-ref" command, but that command is supported by the 1.7.2 version too, and possibly even older versions. * Reduce output from drone, add comments Reduce steps, concatenating them in logical steps * Interrupt step upon first failure * Add Dockerfile for use with ci * Use ad-hoc docker image for testing git-1.7.2 * Avoid running build/vet/clean twice * Set HEAD ref also in testing-1-7 step
I started to write some tests for modules/base/tool.
I'm going to add more tomorrow.