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.

Sign up for GitHub

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

Fix compile failed without gcc #3130

Merged
merged 3 commits into from
Dec 10, 2017
Merged

Fix compile failed without gcc #3130

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
f255f4a
fix comile failed without gcc
lunny Dec 10, 2017
3373fab
add sqlite tag for unit test
lunny Dec 10, 2017
51ec09f
Merge branch 'master' into lunny/fix_compile_without_gcc
lafriks Dec 10, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 3 additions & 2 deletions Makefile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -131,7 +131,8 @@ fmt-check:

.PHONY: test
test:
$(GO) test $(PACKAGES)
$(GO) build # test if go build succeed without sqlite support
Copy link
Member

@ethantkoenig ethantkoenig Dec 10, 2017 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is this under test? IMO, the make test target should run unit tests, nothing more.

Also, building without the sqlite tag already runs in CI (make build), so why is doing it again necessary?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

make build will always add -tags=sqlite. This is a test for building without sqlite. Since all test now always use sqlite.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay, I see why building again, without the sqlite tag, is necessary. However, could we please not put this in the test make target, for the sake of keeping our make targets straightforward? One option would be to just run make build an additional time in CI (without the sqlite tag).

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Or Maybe a new section named comiple-test or build-test?

$(GO) test -tags=sqlite $(PACKAGES)

.PHONY: coverage
coverage:
Expand All @@ -142,7 +143,7 @@ coverage:

.PHONY: unit-test-coverage
unit-test-coverage:
for PKG in $(PACKAGES); do $(GO) test -cover -coverprofile $$GOPATH/src/$$PKG/coverage.out $$PKG || exit 1; done;
for PKG in $(PACKAGES); do $(GO) test -tags=sqlite -cover -coverprofile $$GOPATH/src/$$PKG/coverage.out $$PKG || exit 1; done;

.PHONY: test-vendor
test-vendor:
Expand Down
1 change: 0 additions & 1 deletion models/unit_tests.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@ import (
"github.com/Unknwon/com"
"github.com/go-xorm/core"
"github.com/go-xorm/xorm"
_ "github.com/mattn/go-sqlite3" // for the test engine
"github.com/stretchr/testify/assert"
"gopkg.in/testfixtures.v2"
)
Expand Down
1 change: 0 additions & 1 deletion modules/test/context_tests.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@ import (
"code.gitea.io/gitea/modules/context"

"github.com/go-macaron/session"
_ "github.com/mattn/go-sqlite3" // for the test engine
"github.com/stretchr/testify/assert"
"gopkg.in/macaron.v1"
)
Expand Down