-
-
Notifications
You must be signed in to change notification settings - Fork 13.5k
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
nixos/ejabberd: Fix tests #97526
nixos/ejabberd: Fix tests #97526
Conversation
I believe only nixpkgs-maintainers can ping teams 😦 |
That’s a bit sad 😛 |
How does this relate to #90462? |
@ajs124 : probably a change in default activated modules? |
@ajs124: note that it could be broken for a quite long time and noone noticed/fixed before, I don’t know how to find that kind of history in hydra. I also know that slixmpp made big changes "recently" (more like one year ago) because I had to adapt a few of my python scripts that I use in other projects, which would explain the change in the client part too |
Sorry, I thought the PR you linked was only a version bump that was already merged. So now I can properly answer your question: this PR fixes a part of the PR, i.e. the "test upload part", which doesn’t need to be disabled anymore |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
test script finished in 35.24s
cleaning up
killing client (pid 31)
killing server (pid 9)
(0.00 seconds)
/nix/store/8k1h5vv9gciwlsbi7s3qwh2ig16i0gjp-vm-test-run-ejabberd
The commit message does not adhere to the format specified in https://github.com/NixOS/nixpkgs/blob/master/.github/CONTRIBUTING.md. LGTM otherwise, though. |
@ajs124 oups, I fixed the title of the commit, is it better that way? (I’m not sure if I’m supposed to put "ejabberd" or "nixos/ejabberd" or "nixos/tests/ejabberd" at the beginning of the title, I followed what I saw from other recent similar PR’s) |
"ejabberd:" is fine IMO, it's obvious that the commit is about nixos tests. Lower case after column btw. |
This commit fixes the ejabberd tests for hydra: mod_http_upload and mod_disco need to be explicitly enabled, and a handler needs to be setup to make it work. Also, the client needs to be able to contact the server. The commit also fixes the situation where http upload failed: in that case the client would wait forever because nothing catched the error. Finally, there remains a non-reproducible error where ejabberd server fails to start with an error like: format: "Failed to create cookie file '/var/lib/ejabberd/.erlang.cookie': eacces" (happens ~15%) I tried to check existence of /var/lib/ejabberd/ in pre-start script and saw nothing that would explain this error, so I gave up about this error in particular.
I fixed this one too |
Good commit message btw! |
Thanks :) |
Nice catch for the timeout, thanks! @GrahamcOfBorg test ejabberd |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM.
Thanks!
Motivation for this change
This commit fixes the ejabberd tests for hydra as part of ZHF:
ZHF: #97479
mod_http_upload and mod_disco need to be explicitly enabled, and a
handler needs to be setup to make it work. Also, the client needs to be
able to contact the server.
The commit also fixes the situation where http upload failed: in that
case the client would wait forever because nothing catched the error.
Finally, there remains a non-reproducible error where ejabberd server
fails to start with an error like:
format: "Failed to create cookie file '/var/lib/ejabberd/.erlang.cookie': eacces"
(happens ~15%) I tried to check existence of /var/lib/ejabberd/ in
pre-start script and saw nothing that would explain this error, so I
gave up about this error in particular.
Things done
sandbox
innix.conf
on non-NixOS linux)nix-shell -p nixpkgs-review --run "nixpkgs-review wip"
./result/bin/
)nix path-info -S
before and after)CC @NixOS/nixos-release-managers