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

tests: Fix the start of the daemon #6179

Merged
merged 1 commit into from
Feb 28, 2022
Merged

tests: Fix the start of the daemon #6179

merged 1 commit into from
Feb 28, 2022

Conversation

thufschmitt
Copy link
Member

  • Make sure that it starts even without the nix-command xp feature
  • Fail if it doesn’t manage to start

This fixes a 30s wait for every test in init.sh as the daemon couldn’t start, but the code was just waiting 30s and continuing as if everything was all right.

@thufschmitt
tests: Fix the start of the daemon
b8f8aef 
- Make sure that it starts even without the `nix-command` xp feature
- Fail if it doesn’t manage to start

This fixes a 30s wait for every test in `init.sh` as the daemon couldn’t
start, but the code was just waiting 30s and continuing as if everything
was all right.
@thufschmitt
Copy link
Member Author

This fixes a 30s wait for every test in init.sh as the daemon couldn’t start, but the code was just waiting 30s and continuing as if everything was all right

Which in practice means that the CA is faster by something between 20 and 40 mins :)

@Ericson2314
Copy link
Member

@thufschmitt this overlaps a bit with my #5753, I guess I will rebase that but I hope we can do both.

@thufschmitt
Copy link
Member Author

@thufschmitt this overlaps a bit with my #5753, I guess I will rebase that but I hope we can do both.

Ah, I knew you had something touching the same bits of code :) (and looks like yours would fix the issue too).

If you’re OK rebasing, I’ll merge that one right now as the CI times are really a pain and that’s the quickest fix

@Ericson2314
Copy link
Member

@thufschmitt Yes I am totally fine rebasing. Sound good!

This was referenced Feb 28, 2022
Merged
Merged
Ericson2314
Ericson2314 approved these changes Feb 28, 2022
View reviewed changes
Copy link
Member

@Ericson2314 Ericson2314 left a comment

Choose a reason for hiding this comment

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

Ready to merge

@thufschmitt thufschmitt merged commit 1c98542 into master Feb 28, 2022
@thufschmitt thufschmitt deleted the properly-start-the-daemon-in-tests branch February 28, 2022 18:23
@nixos-discourse
Copy link

This pull request has been mentioned on NixOS Discourse. There might be relevant details there:

https://discourse.nixos.org/t/tweag-nix-dev-update-25/18003/1

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Ericson2314 Ericson2314 Ericson2314 approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@thufschmitt @Ericson2314 @nixos-discourse