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

CI: test_plugin_systemd.rb - fixups for intermittent retries #3594

Merged
merged 3 commits into from
Jan 11, 2025

Conversation

MSP-Greg
Copy link
Member

@MSP-Greg MSP-Greg commented Jan 3, 2025

Description

The tests in this file fail and/or retry intermittently. The changes involve code that parses the notify socket. On GHA CI, there are status messages interlaced with other messages. These are due to delays common on GHA, but not reproducible locally.

It seems to help...

Your checklist for this pull request

  • I have reviewed the guidelines for contributing to this repository.
  • I have added (or updated) appropriate tests if this PR fixes a bug or adds a feature.
  • My pull request is 100 lines added/removed or less so that it can be easily reviewed.
  • If this PR doesn't need tests (docs change), I added [ci skip] to the title of the PR.
  • If this closes any issues, I have added "Closes #issue" to the PR description or my commit messages.
  • I have updated the documentation accordingly.
  • All new and existing tests passed, including Rubocop.

Sorry, something went wrong.

@github-actions github-actions bot added the waiting-for-review Waiting on review from anyone label Jan 3, 2025
@@ -137,10 +137,7 @@ def self.notify(state, unset_env=false)
ENV.delete("NOTIFY_SOCKET") if unset_env

begin
Addrinfo.unix(sock, :DGRAM).connect do |s|
s.close_on_exec = true
Copy link
Member

Choose a reason for hiding this comment

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

I assume we remove this line because (from https://docs.ruby-lang.org/en/3.3/IO.html#method-i-close_on_exec-3D)

Ruby sets close-on-exec flags of all file descriptors by default since Ruby 2.0.0. So you don’t need to set by yourself.

@socket = Socket.new(:UNIX, :DGRAM, 0)
socket_ai = Addrinfo.unix(@sockaddr)
@socket.bind Addrinfo.unix(@sockaddr)
@env = {"NOTIFY_SOCKET" => @sockaddr }
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
@env = {"NOTIFY_SOCKET" => @sockaddr }
@env = { "NOTIFY_SOCKET" => @sockaddr }

Copy link
Member Author

Choose a reason for hiding this comment

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

As you suggested, and rebased.

Interesting, much of Ruby's docs have no spaces with hashes, but much of the Rails docs use spaces...

@github-actions github-actions bot added waiting-for-merge and removed waiting-for-review Waiting on review from anyone labels Jan 10, 2025

Verified

This commit was signed with the committer’s verified signature.
drazisil-codecov Joe Becher
@MSP-Greg MSP-Greg force-pushed the 00-test_plugin_systemd.rb branch from 81c92cc to 872f84b Compare January 11, 2025 02:42
@MSP-Greg MSP-Greg merged commit edd2a17 into puma:master Jan 11, 2025
79 of 87 checks passed
@MSP-Greg MSP-Greg deleted the 00-test_plugin_systemd.rb branch January 11, 2025 03:28
joshuay03 pushed a commit to joshuay03/puma that referenced this pull request Jan 12, 2025
* CI: test_plugin_systemd.rb - update systemd NOTIFY_SOCKET creation

* CI: test_plugin_systemd.rb - refactor assert_message

* sd_notify.rb - small refactor, update for new Rubies
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants