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

Prefer rackup file specified by CLI #2236

Merged

Conversation

acmh
Copy link
Contributor

@acmh acmh commented Apr 25, 2020

Description

This PR aims to always give preference for the rackup file specified by the CLI. I also took opportunity and added some tests.

This Closes #2225 .

Your checklist for this pull request

  • I have reviewed the guidelines for contributing to this repository.
  • I have added an entry to History.md if this PR fixes a bug or adds a feature. If it doesn't need an entry to HISTORY.md, I have added [changelog skip] the pull request title.
  • I have added 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.

@acmh
Copy link
Contributor Author

acmh commented Apr 25, 2020

cc @nateberkopec

@@ -43,6 +43,19 @@ def test_term_suppress
assert_equal 0, status
end

def test_prefer_rackup_file_specified_by_cli
skip_unless_signal_exist? :TERM
skip_on :jruby
Copy link
Member

Choose a reason for hiding this comment

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

You can remove this line

_, status = stop_server

assert_match("Hello World", reply)
assert_equal 15, status
Copy link
Member

Choose a reason for hiding this comment

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

This assertion isn't necessary I think, we only need the correct reply.

@nateberkopec
Copy link
Member

Thank you so much!

Can you confirm this new test fails on master?

@nateberkopec nateberkopec added bug waiting-for-changes Waiting on changes from the requestor labels Apr 27, 2020
@acmh
Copy link
Contributor Author

acmh commented Apr 27, 2020

All changes requested were made.

Thank you so much!

Can you confirm this new test fails on master?

Yes, I confirm.

lib/puma/dsl.rb Outdated Show resolved Hide resolved
Co-Authored-By: Cristian Rivera <cristian_rivera@me.com>
@nateberkopec nateberkopec merged commit 5a4e744 into puma:master Apr 27, 2020
@rmachielse
Copy link

Is it possible to backport this to 4.3? We are experiencing this issue with puma 4.3 too.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug waiting-for-changes Waiting on changes from the requestor
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Rackup overrides CLI preference for app
4 participants