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.

Sign up for GitHub

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

Jump to bottom

nixos/ejabberd: Fix tests #97526

Merged
merged 1 commit into from
Sep 10, 2020
Merged

nixos/ejabberd: Fix tests #97526

merged 1 commit into from
Sep 10, 2020

Conversation

immae
Copy link
Contributor

@immae immae commented Sep 9, 2020

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
  • Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS linux)
  • Built on platform(s)
    • NixOS
    • macOS
    • other Linux distributions
  • Tested via one or more NixOS test(s) if existing and applicable for the change (look inside nixos/tests)
  • Tested compilation of all pkgs that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review wip"
  • Tested execution of all binary files (usually in ./result/bin/)
  • Determined the impact on package closure size (by running nix path-info -S before and after)
  • Ensured that relevant documentation is up to date
  • Fits CONTRIBUTING.md.

CC @NixOS/nixos-release-managers

@ofborg ofborg bot added 6.topic: nixos Issues or PRs affecting NixOS modules, or package usability issues specific to NixOS 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin 10.rebuild-linux: 0 This PR does not cause any packages to rebuild on Linux labels Sep 9, 2020
@dasJ
Copy link
Member

dasJ commented Sep 9, 2020

Since @immae cannot ping: @NixOS/nixos-release-managers

From IRC:

immae | I tried to follow the guidelines for ZHF ( #97479 ) but github doesn’t let me ping @NixOS/nixos-release-managers : #97526 what can I do?

@worldofpeace
Copy link
Contributor

I believe only nixpkgs-maintainers can ping teams 😦

@immae
Copy link
Contributor Author

immae commented Sep 9, 2020

I believe only nixpkgs-maintainers can ping teams frowning

That’s a bit sad 😛

@ajs124
Copy link
Member

ajs124 commented Sep 9, 2020

How does this relate to #90462?

@immae
Copy link
Contributor Author

immae commented Sep 9, 2020

@ajs124 : probably a change in default activated modules?

@immae
Copy link
Contributor Author

immae commented Sep 9, 2020

@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

@immae
Copy link
Contributor Author

immae commented Sep 9, 2020

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

Atemu
Atemu approved these changes Sep 9, 2020
View reviewed changes
Copy link
Member

@Atemu Atemu left a 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

@ajs124
Copy link
Member

ajs124 commented Sep 9, 2020

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 ajs124 mentioned this pull request Sep 9, 2020
Closed
10 tasks
@immae
Copy link
Contributor Author

immae commented Sep 9, 2020

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)

@Atemu
Copy link
Member

Atemu commented Sep 9, 2020

"ejabberd:" is fine IMO, it's obvious that the commit is about nixos tests.

Lower case after column btw.

@immae
ejabberd: fix failing tests
cdaec7e 
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.
@immae
Copy link
Contributor Author

immae commented Sep 9, 2020

Lower case after column btw.

I fixed this one too

@Atemu
Copy link
Member

Atemu commented Sep 9, 2020

Good commit message btw!

@immae
Copy link
Contributor Author

immae commented Sep 9, 2020

Good commit message btw!

Thanks :)

@picnoir
Copy link
Member

picnoir commented Sep 10, 2020 •
edited
Loading

Nice catch for the timeout, thanks!

@GrahamcOfBorg test ejabberd

picnoir
picnoir approved these changes Sep 10, 2020
View reviewed changes
Copy link
Member

@picnoir picnoir left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM.

Thanks!

@picnoir picnoir changed the title Fix ejabberd tests nixos/ejabberd: Fix tests Sep 10, 2020
@picnoir picnoir merged commit a4a1c01 into NixOS:master Sep 10, 2020
@immae immae mentioned this pull request Sep 10, 2020
Merged
10 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@picnoir picnoir picnoir approved these changes

@Atemu Atemu Atemu approved these changes

Assignees
No one assigned
Labels
6.topic: nixos Issues or PRs affecting NixOS modules, or package usability issues specific to NixOS 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin 10.rebuild-linux: 0 This PR does not cause any packages to rebuild on Linux
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@immae @dasJ @worldofpeace @ajs124 @Atemu @picnoir