[TEST] increase test coverage of package runtime to maximum #75

genofire · 2017-10-05T13:26:17Z

No description provided.

corny · 2017-10-05T14:00:47Z

webserver/webserver_test.go

+	srv := New(":8080", "/tmp")
+	assert.NotNil(srv)
+
+	//TODO test panic in a go routine (HOWTO)


Please also stop the server.

Closing the webserver create a panic 👎 -> so this tests fails

Then fix it!

http.Server in Go 1.8 supports graceful shutdown

genofire · 2017-10-05T14:40:52Z

cmd/serve.go

@@ -38,7 +38,8 @@ var serveCmd = &cobra.Command{
 		if config.Webserver.Enable {
 			log.Println("starting webserver on", config.Webserver.Bind)
 			srv := webserver.New(config.Webserver.Bind, config.Webserver.Webroot)
-			go srv.Close()
+			go webserver.Start(srv)
+			defer srv.Close()


Sag mal, war das ein Bug mit defer vs. go ? @corny

go srv.Close() sieht falsch aus.

genofire · 2017-10-07T11:16:14Z

@corny magstdu es dir durchschauen und ggf. squash mergen?

corny · 2017-10-07T11:54:52Z

webserver/webserver_test.go

+	assert := assert.New(t)
+
+	srv := New(":8080", "/tmp")
+	assert.NotNil(srv)


Bitte den Server Socket wieder schließen im Test!

ups, stimmt, da war noch was

Das ist scheiße. Wenn der Server ordentlich beendet wird, darf es kein panic geben!

Die Lösung ist wahrscheinlich hier:
https://talks.golang.org/2017/state-of-go/stdlib/shutdown.go

Ah cool, die haben die Errors als const definiert 👍
Done

corny · 2017-10-09T23:28:16Z

webserver/webserver_test.go

+	assert := assert.New(t)
+
+	srv := New(":8080", "/tmp")
+	assert.NotNil(srv)


Das ist scheiße. Wenn der Server ordentlich beendet wird, darf es kein panic geben!

corny · 2017-10-11T14:33:03Z

webserver/webserver.go

+func Start(srv *http.Server) {
+	// service connections
+	if err := srv.ListenAndServe(); err != nil {
+		if err != http.ErrServerClosed {
 			panic(err)


ListenAndServe() gibt niemals nil zurück. Bitte vereinfachen.

coveralls · 2017-10-11T15:57:15Z

Coverage increased (+1.5%) to 57.117% when pulling 5ed98f3 on increase-testing into 3649bdd on master.

genofire · 2017-10-12T12:03:20Z

Soll ich schon mal squashen? Oder magst zum verfolgen noch die einzelnen commit haben

genofire added the wip label Oct 5, 2017

genofire self-assigned this Oct 5, 2017

genofire requested a review from corny October 5, 2017 13:26

genofire force-pushed the increase-testing branch from 922ec8d to 36d881b Compare October 5, 2017 13:27

FreifunkBremen deleted a comment from coveralls Oct 5, 2017

genofire changed the title ~~[TEST] runtime config to 100%~~ [TEST] increase test coverage of package runtime to 100% Oct 5, 2017

genofire force-pushed the increase-testing branch from cb62a1d to bb59974 Compare October 5, 2017 13:53

FreifunkBremen deleted a comment from coveralls Oct 5, 2017

corny requested changes Oct 5, 2017

View reviewed changes

genofire force-pushed the increase-testing branch from 44a9928 to cb8b3de Compare October 5, 2017 14:24

FreifunkBremen deleted a comment from coveralls Oct 5, 2017

genofire commented Oct 5, 2017

View reviewed changes

FreifunkBremen deleted a comment from coveralls Oct 5, 2017

FreifunkBremen deleted a comment from coveralls Oct 7, 2017

genofire added review and removed wip labels Oct 7, 2017

genofire changed the title ~~[TEST] increase test coverage of package runtime to 100%~~ [TEST] increase test coverage of package runtime to maximum Oct 7, 2017

corny requested changes Oct 7, 2017

View reviewed changes

FreifunkBremen deleted a comment from coveralls Oct 7, 2017

corny requested changes Oct 9, 2017

View reviewed changes

FreifunkBremen deleted a comment from coveralls Oct 11, 2017

corny requested changes Oct 11, 2017

View reviewed changes

genofire force-pushed the increase-testing branch from a9dfe30 to b72f35d Compare October 11, 2017 15:53

FreifunkBremen deleted a comment from coveralls Oct 11, 2017

FreifunkBremen deleted a comment from coveralls Oct 12, 2017

genofire force-pushed the increase-testing branch from b72f35d to 5705b3d Compare October 12, 2017 13:21

genofire added a commit that referenced this pull request Oct 12, 2017

[TEST] increase test coverage of package runtime to maximum #75

5705b3d

[TEST] increase test coverage of package runtime to maximum (#75)

7865ab1

genofire force-pushed the increase-testing branch from 5705b3d to 7865ab1 Compare October 12, 2017 13:22

corny merged commit b892105 into master Oct 12, 2017

corny deleted the increase-testing branch October 12, 2017 13:25

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[TEST] increase test coverage of package runtime to maximum #75

[TEST] increase test coverage of package runtime to maximum #75

genofire commented Oct 5, 2017

corny Oct 5, 2017

genofire Oct 5, 2017

corny Oct 5, 2017

corny Oct 5, 2017

genofire Oct 5, 2017

corny Oct 5, 2017

genofire commented Oct 7, 2017

corny Oct 7, 2017

genofire Oct 7, 2017

genofire Oct 7, 2017

corny Oct 9, 2017

corny Oct 9, 2017

genofire Oct 11, 2017

corny Oct 9, 2017

corny Oct 11, 2017

coveralls commented Oct 11, 2017 •

edited

Loading

genofire commented Oct 12, 2017

[TEST] increase test coverage of package runtime to maximum #75

[TEST] increase test coverage of package runtime to maximum #75

Conversation

genofire commented Oct 5, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

genofire commented Oct 7, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

coveralls commented Oct 11, 2017 • edited Loading

genofire commented Oct 12, 2017

coveralls commented Oct 11, 2017 •

edited

Loading