Skip to content

Conversation

@hramrach
Copy link
Contributor

@hramrach hramrach commented Dec 2, 2025

Missing make file dependency:

git clone https://github.com/go-gitea/gitea.git
make -C gitea test-sqlite

go test -c code.gitea.io/gitea/tests/integration -o integrations.sqlite.test -tags ' sqlite sqlite_unlock_notify'
sed -e 's|{{REPO_TEST_DIR}}||g'
-e 's|{{TEST_LOGGER}}|test,file|g'
-e 's|{{TEST_TYPE}}|integration|g'
tests/sqlite.ini.tmpl > tests/sqlite.ini
GITEA_ROOT="/home/hramrach/gitea" GITEA_CONF=tests/sqlite.ini ./integrations.sqlite.test
Could not find gitea binary at /home/hramrach/gitea/gitea

make: *** [Makefile:506: test-sqlite] Error 1

@GiteaBot GiteaBot added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label Dec 2, 2025
@hramrach hramrach changed the title Testdep Add missing dependency of database tests on backend Dec 2, 2025
@GiteaBot GiteaBot added lgtm/need 1 This PR needs approval from one additional maintainer to be merged. and removed lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. labels Dec 2, 2025
@silverwind
Copy link
Member

CI failures are unrelated and will be fixed with #36074.

@silverwind
Copy link
Member

CI failures are unrelated and will be fixed with #36074.

Actually no, not entirely unrelated. go: -race requires cgo; enable cgo by setting CGO_ENABLED=1 is related.

delvh
delvh previously approved these changes Dec 3, 2025
@GiteaBot GiteaBot added lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. and removed lgtm/need 1 This PR needs approval from one additional maintainer to be merged. labels Dec 3, 2025
@wxiaoguang
Copy link
Contributor

Well, I don't think this "fix" is right and can be approved as the current implementation.

@silverwind @delvh , see the CI failures. There are more details.

@wxiaoguang wxiaoguang marked this pull request as draft December 3, 2025 04:07
@GiteaBot GiteaBot added lgtm/need 1 This PR needs approval from one additional maintainer to be merged. and removed lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. labels Dec 3, 2025
@hramrach
Copy link
Contributor Author

hramrach commented Dec 3, 2025

Interesting.

Clearly the dependency is real because the workflow in question first builds backend, and then runs the test.

However, the test is run with environment variables set that fail the backend build.

My suggestion would be to drop the CGO_ENABLED=0 which would then make the backend buildable as part of the test but I am not expert on go and don't know what this was supposed to accomplish.

@hramrach
Copy link
Contributor Author

hramrach commented Dec 3, 2025

I see. cgo, at least the version you use is broken on Arm due to bogus inline assembly.

@silverwind
Copy link
Member

silverwind commented Dec 3, 2025

I think we may just need to add CGO_ENABLED = 1 after below line because the race detector depends on it and RACE_ENABLED should only ever be set during test runs:

gitea/Makefile

Line 93 in 46d7ade

GOTESTFLAGS += -race

@hramrach hramrach force-pushed the testdep branch 5 times, most recently from 42ab49e to 7835f8b Compare December 3, 2025 23:07
CGO is required for tests that use -race as well as some other build
options.

On arm64 the build fails when CGO is enabled because of some assembly
error.
fixes this error:

 make test-sqlite
go test  -c code.gitea.io/gitea/tests/integration -o integrations.sqlite.test -tags ' sqlite sqlite_unlock_notify'
sed -e 's|{{REPO_TEST_DIR}}||g' \
	-e 's|{{TEST_LOGGER}}|test,file|g' \
	-e 's|{{TEST_TYPE}}|integration|g' \
		tests/sqlite.ini.tmpl > tests/sqlite.ini
GITEA_ROOT="/home/hramrach/gitea" GITEA_CONF=tests/sqlite.ini ./integrations.sqlite.test
Could not find gitea binary at /home/hramrach/gitea/gitea

make: *** [Makefile:506: test-sqlite] Error 1
No need to build it explicitly, db tests now depend on it.
@hramrach hramrach marked this pull request as ready for review December 4, 2025 10:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

lgtm/need 1 This PR needs approval from one additional maintainer to be merged. modifies/internal

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants