karlseguin / pg.zig

Native PostgreSQL driver / client for Zig
MIT License
214 stars 16 forks source link

std.Build: try future std.Build API #25

Open BratishkaErik opened 3 months ago

BratishkaErik commented 3 months ago

Hello, I wanted to collect some data for myself on https://www.github.com/ziglang/zig/pull/20388 (not merged at the moment of writing) can simplify logic in build.zig for many existing packages. This package was chosen as the first test subject; I'm kindly asking what do you think about these changes.

BTW not related to this changes, but I have noticed some test failures that are (IIUC) related to localized messages: test expects message in English but Postgres instance on my notebook returns message in Russian:

====== expected this output: =========
password authentication failed for user "does_not_exist"␃

======== instead found this: =========
роль "does_not_exist" не существует␃

======================================
First difference occurs on line 1:
expected:
password authentication failed for user "does_not_exist"
^ ('\x70')
found:
роль "does_not_exist" не существует
^ ('\xd1')

================================================================================
"Conn: auth unknown user" - TestExpectedEqual
================================================================================
/home/bratishkaerik/github.com/zig/lib/std/testing.zig:624:9: 0x1095d00 in expectEqualStrings (test)
        return error.TestExpectedEqual;
        ^
/home/bratishkaerik/github.com/pg.zig/src/conn.zig:378:2: 0x10b14a4 in test.Conn: auth unknown user (test)
 try t.expectString("password authentication failed for user \"does_not_exist\"", conn.err.?.message);
 ^
Conn: auth unknown user (26.85ms)

Here's full log: tests.log

karlseguin commented 3 months ago

Looks simpler. Seems like an improvement and a nice step forward for newcomers.

karlseguin commented 3 months ago

As for the failed tests due to localization, I changed the docker image to force en-US: https://github.com/karlseguin/pg.zig/blob/master/tests/Dockerfile

So if you're testing using that image, by first running:

docker build tests/ -f tests/Dockerfile -t "pgzig:pg"

and then starting the container via:

make d

the tests should pass