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.

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

lighttpd: update to 1.4.73 #46978

Merged
merged 1 commit into from
Oct 31, 2023
Merged

Conversation

gstrauss
Copy link
Contributor

lighttpd: update to 1.4.73

Testing the changes

I tested the changes in this PR: NO

(build-tested elsewhere: on openwrt)

@classabbyamp classabbyamp added the needs-testing Testing a PR or reproducing an issue needed label Oct 31, 2023
@classabbyamp
Copy link
Member

classabbyamp commented Oct 31, 2023

next time just amend and force-push the previous PR instead of opening another one

I tested the changes in this PR: NO
(build-tested elsewhere: on openwrt)

this PR isn't very useful if it wasn't run-tested at all and wasn't built on void (or do you mean in xbps-src on openwrt?)

@gstrauss
Copy link
Contributor Author

gstrauss commented Oct 31, 2023

@classabbyamp I am a lighttpd developer. I have tested this release on multiple platforms (Linux, FreeBSD, OpenBSD, NetBSD, MacOS) and multiple architectures (under openwrt), though not specifically on void linux. If this PR is not acceptable, then please close it.

@gstrauss
Copy link
Contributor Author

gstrauss commented Oct 31, 2023

this PR isn't very useful if it wasn't run-tested at all and wasn't built on void (or do you mean in xbps-src on openwrt?)

The CI tests built the package on multiple architectures under void linux. Is that not sufficient for build testing?

Regarding run-testing: lighttpd source supports make check to run the lighttpd test suite. This is exercised as part of some distro CI processes. Can void linux CI do this?

@gstrauss
Copy link
Contributor Author

@classabbyamp the void linux CI does perform lighttpd make check, as I can see from the build logs:

=> lighttpd-1.4.73_1: running do_check ...
ninja: Entering directory `build'
[0/1] Running all tests.
1/9 test_configfile  OK              0.01s
2/9 test_common      OK              0.01s
3/9 test_mod         OK              0.01s
4/9 prepare          OK              0.01s
5/9 request.t        OK              0.26s
6/9 core-condition.t OK              0.09s
7/9 mod-fastcgi.t    OK              0.10s
8/9 mod-scgi.t       OK              0.08s
9/9 cleanup          OK              0.01s

@classabbyamp
Copy link
Member

The CI tests built the package on multiple architectures under void linux. Is that not sufficient for build testing?

it is, but running it on void is the more important testing

Can void linux CI do this?

yes, it runs check on all non-cross arches

In general, we'd prefer the package get tested in a real void environment by someone who uses the package on void (in addition to the CI and make check), as that can shake out some kinds of bugs and incompatibilities that the other parts of testing would not catch

@classabbyamp
Copy link
Member

If someone comes along and tests it in situ that's fine too, this PR doesn't need to be closed

@gstrauss
Copy link
Contributor Author

Sounds like you think it more important for joe-user to say "yep, lighttpd serves my static file index.html on void linux." than the more extensive lighttpd test suite, which performs both unit tests and integration tests (e.g. full web request to a fastcgi server, an scgi server, CGI scripts, and more).

For a different package with minimal tests, I could see the desire for in-person manual testing of a package. For more mature packages with more extensive test suites, I find that argument to be weaker that a user manually running a few smoke tests is better than the mature test suite which runs on multiple distros and multiple platforms and multiple architectures.

@0x5c
Copy link
Contributor

0x5c commented Oct 31, 2023

Tested here and it works

Sounds like you think it more important for joe-user to say "yep, lighttpd serves my static file index.html on void linux." than the more extensive lighttpd test suite, which performs both unit tests and integration tests (e.g. full web request to a fastcgi server, an scgi server, CGI scripts, and more).

It's && not ||; test failure is fatal in CI and will block merging of a PR until the cause can be found and fixed, unless the test is found to be broken in the first place or needing fixtures that can't run in CI.

Manual testing is still required because of the variety of ways different packages do tests, and the extra work that would be required to verify that each one of them has enough tests and the right kind of tests that would catch runtime problems.

It is also important to check that things like the services Void ships for packages do in fact continue to work as packaged after the update.

@classabbyamp classabbyamp merged commit 423a7d0 into void-linux:master Oct 31, 2023
8 checks passed
@classabbyamp classabbyamp removed the needs-testing Testing a PR or reproducing an issue needed label Oct 31, 2023
@gstrauss
Copy link
Contributor Author

[...] test failure is fatal in CI and will block merging of a PR until the cause can be found and fixed [...]

@0x5c of course. My responses in this PR are due to @classabbyamp saying

this PR isn't very useful if it wasn't run-tested at all and wasn't built on void (or do you mean in xbps-src on openwrt?)

Do you agree with @classabbyamp? Do you not want encourage submissions to maintain packages?

It is also important to check that things like the services Void ships for packages do in fact continue to work as packaged after the update.

@0x5c Can that be added to the CI? If a package includes a service file, there can be an automated test which starts and stops the service.

If I spin up a void VM, what is the "manual testing" that you suggest before I say "I tested the changes in this PR: yes" in the PR? (You're trusting people not to lie here. I am at least asking what should be "manually" tested, since I can probably automate it.)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants