Skip to content

Add unit tests for XMLRPC::CGIServer #60

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

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Conversation

herwinw
Copy link
Member

@herwinw herwinw commented Apr 29, 2025

The implementation is mostly a copy-paste from the webrick server test, including the code style.
Because this needs a CGI server, it still has a dependency on webrick to run that server. This setup is a bit slow, on my system the overall execution time of the tests jumps from 1 second to 7 seconds.

The do not work out of the box, I'm sure this can be fixed but I have no idea
how to do that.
@herwinw
Copy link
Member Author

herwinw commented Apr 29, 2025

I've disabled these new tests on Windows platforms, the CGI structure needs an executable file and the usual #!/usr/bin/env ruby and chmod +x does not work on Windows. This can probably be fixed, but I have no idea how.

end

def test_client_server
omit("The CGI file does not work on Windows") if RUBY_PLATFORM =~ /(mswin|mingw)/
Copy link
Member

Choose a reason for hiding this comment

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

We can use Gem.win_platform?:

Suggested change
omit("The CGI file does not work on Windows") if RUBY_PLATFORM =~ /(mswin|mingw)/
omit("The CGI file does not work on Windows") if Gem.win_platform?

tempfile.close
File.chmod(0755, tempfile.path)

[false].each do |use_ssl|
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
[false].each do |use_ssl|
use_ssl = false


[false].each do |use_ssl|
option = setup_http_server_option(use_ssl)
with_server(option, WEBrick::HTTPServlet::CGIHandler, tempfile.path) {|addr|
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
with_server(option, WEBrick::HTTPServlet::CGIHandler, tempfile.path) {|addr|
with_server(option, WEBrick::HTTPServlet::CGIHandler, tempfile.path) do |addr|


def cgi_bin_script
<<~RUBY
#!/usr/bin/env ruby
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
#!/usr/bin/env ruby
#!#{Gem.ruby}

end

s.set_default_handler do |name, *args|
raise XMLRPC::FaultException.new(-99, "Method #{name} missing" +
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
raise XMLRPC::FaultException.new(-99, "Method #{name} missing" +
raise XMLRPC::FaultException.new(-99, "Method \#{name} missing" +


s = XMLRPC::CGIServer.new

s.add_handler("test.add") do |a,b|
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
s.add_handler("test.add") do |a,b|
s.add_handler("test.add") do |a, b|

a + b
end

s.add_handler("test.div") do |a,b|
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
s.add_handler("test.div") do |a,b|
s.add_handler("test.div") do |a, b|

assert_equal param, 2

# introspection
assert_equal ["test.add", "test.div", "system.listMethods", "system.methodSignature", "system.methodHelp"], @s.call("system.listMethods")
Copy link
Member

Choose a reason for hiding this comment

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

Could you avoid omitting parenthesis for assertions?
See also: https://bugs.ruby-lang.org/issues/20922

Could you align long expected and actual for readability?

Suggested change
assert_equal ["test.add", "test.div", "system.listMethods", "system.methodSignature", "system.methodHelp"], @s.call("system.listMethods")
assert_equal(["test.add", "test.div", "system.listMethods", "system.methodSignature", "system.methodHelp"],
@s.call("system.listMethods"))

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants