ci-test: Fix syntax/formatting check so it actually emits an exit code

[ottok]

I noticed this bug while working on #225 and I saw this:

https://github.com/Debian/dh-make-golang/actions/runs/12617186618/job/35159206853

The old format didn't emit any exit codes and thus did not cause CI to fail:

$ diff -u <(echo -n) <(gofmt -d -s .)
make.go:513:46: missing ',' before newline in composite literal
$ echo $?
0

The smallest possible fix to this is to ensure gofmt output actually goes to stdout as the original test expected:

$ diff -u <(echo -n) <(gofmt -d -s . 2>&1)
--- /dev/fd/63	2025-01-05 07:46:28.550632333 +0000
+++ /dev/fd/62	2025-01-05 07:46:28.550632333 +0000
@@ -0,0 +1 @@
+make.go:513:46: missing ',' before newline in composite literal
$ echo $?
1

[ottok]

Thanks @tianon and @zhsj for commenting my other PRs. Could you review/approve this one too? Thanks

[ottok]

@Maytha8 or @n-peugnet Can you review/approve this?

[n-peugnet]

Looks OK, but gofmt is more about code formatting/prettify than syntax errors. In this case the syntax errors are caught in the next step, so this is more or less the same thing as before.

[ottok]

Looks OK, but gofmt is more about code formatting/prettify than syntax errors. In this case the syntax errors are caught in the next step, so this is more or less the same thing as before.

If the step is useless, should we remove it instead of fixing it?

[n-peugnet]

If the step is useless, should we remove it instead of fixing it?

This step is not useless, it checks that the code is formatted with gofmt.

[ottok]

But it is part of a CI pipeline, so if the job can and will never fail, nobody will ever look at it.

[n-peugnet]

gofmt -d sends the diff to stdout:

$ diff -u <(echo -n) <(gofmt -d -s .)
--- /dev/fd/63	2025-01-14 19:01:56.993862536 +0100
+++ /dev/fd/62	2025-01-14 19:01:57.005862508 +0100
@@ -0,0 +1,12 @@
+diff main.go.orig main.go
+--- main.go.orig
++++ main.go
+@@ -15,7 +15,7 @@
+ )
+ 
+ func usage() {
+-fmt.Fprintf(os.Stderr, "%s\n", buildVersionString())
++	fmt.Fprintf(os.Stderr, "%s\n", buildVersionString())
+ 	fmt.Fprintf(os.Stderr, "\n")
+ 	fmt.Fprintf(os.Stderr, "%s is a tool that converts Go packages into Debian package source.\n", program)
+ 	fmt.Fprintf(os.Stderr, "\n")
$ echo $?
1

But I agree that the result is not very beautiful with the double diff. In my own projects I usually use go fmt ./... && git diff --exit-code --color, but it does not check for simplifications (-s), for example: https://salsa.debian.org/lintian/lintian-ssg/-/jobs/6488774#L22

Instead we could use gofmt -w -s . && git diff --exit-code --color. I should start using it for my own projects in fact 🤔

[n-peugnet]

Here is an exemple of what it does that just happened to me 😅: https://github.com/Debian/dh-make-golang/actions/runs/12845745463/job/35820145357?pr=243

[ottok]

Updated latest version to use the double command you suggested. It seems like the most solid option. Thanks!