Fix bug hidden on CI and make ci failed if tests failure

[lunny]

The tests on migration tests failed but CI reports successfully

https://github.com/go-gitea/gitea/actions/runs/7364373807/job/20044685969#step:8:141

This PR will fix the bug on migration v283 and also the CI hidden behaviour.

The reason is on the Makefile

GITEA_ROOT="$(CURDIR)" GITEA_CONF=tests/mysql.ini $(GO) test $(GOTESTFLAGS) -tags='$(TEST_TAGS)' $(MIGRATE_TEST_PACKAGES) will return the error exit code.

But

for pkg in $(shell $(GO) list code.gitea.io/gitea/models/migrations/...); do \ GITEA_ROOT="$(CURDIR)" GITEA_CONF=tests/mysql.ini $(GO) test $(GOTESTFLAGS) -tags '$(TEST_TAGS)' $$pkg; \ done

will not work.

This also fix #29602

[silverwind]

Interesting, does make foo bar trigger the bar target and succeed even when foo fails? I would have expected it to exit on first failed target.

[wxiaoguang]

Interesting +1. Is the root problem really fixed?

make failure success should NOT succeed:

bash-3.2$ cat Makefile
a:
	false

b:
	true


bash-3.2$ make a
false
make: *** [a] Error 1
bash-3.2$ echo $?
2


bash-3.2$ make b
true
bash-3.2$ echo $?
0


bash-3.2$ make a b
false
make: *** [a] Error 1
bash-3.2$ echo $?
2

[lunny]

Interesting, does make foo bar trigger the bar target and succeed even when foo fails? I would have expected it to exit on first failed target.

I guess that but unfortunately, it's not right. It must be affected by others.

[KN4CK3R]

@lunny Could you fix the v96 too I mentioned at Discord?

diff --git a/models/migrations/v1_10/v96.go b/models/migrations/v1_10/v96.go
index 34c8240031..2847be57b1 100644
--- a/models/migrations/v1_10/v96.go
+++ b/models/migrations/v1_10/v96.go
@@ -38,7 +38,7 @@ func DeleteOrphanedAttachments(x *xorm.Engine) error {
 			return err
 		}
 		if len(attachments) == 0 {
-			return nil
+			break
 		}
 
 		ids := make([]int64, 0, limit)
@@ -58,7 +58,9 @@ func DeleteOrphanedAttachments(x *xorm.Engine) error {
 			}
 		}
 		if len(attachments) < limit {
-			return nil
+			break
 		}
 	}
+
+	return sess.Commit()
 }

[lunny]

@lunny Could you fix the v96 too I mentioned at Discord?

diff --git a/models/migrations/v1_10/v96.go b/models/migrations/v1_10/v96.go
index 34c8240031..2847be57b1 100644
--- a/models/migrations/v1_10/v96.go
+++ b/models/migrations/v1_10/v96.go
@@ -38,7 +38,7 @@ func DeleteOrphanedAttachments(x *xorm.Engine) error {
 			return err
 		}
 		if len(attachments) == 0 {
-			return nil
+			break
 		}
 
 		ids := make([]int64, 0, limit)
@@ -58,7 +58,9 @@ func DeleteOrphanedAttachments(x *xorm.Engine) error {
 			}
 		}
 		if len(attachments) < limit {
-			return nil
+			break
 		}
 	}
+
+	return sess.Commit()
 }

Looks like there is no sess.Begin, why we need to add sess.Commit()?

[KN4CK3R]

Line 24?

gitea/models/migrations/v1_10/v96.go

Lines 24 to 25 in 16f1326

	sess := x.NewSession()
	defer sess.Close()

Edit: Nvm, yes, no transaction here.

[lunny]

Finally fixed all migration tests. It's ready to re-review now.

[GiteaBot]

I was unable to create a backport for 1.21. @lunny, please send one manually. 🍵

go run ./contrib/backport 29254
...  // fix git conflicts if any
go run ./contrib/backport --continue