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

Allow single blueprint argument #17

Closed

Conversation

rylnd
Copy link
Contributor

@rylnd rylnd commented Aug 14, 2015

@gonzalo-bulnes currently, one is unable to do the following:

Dredd::Rack::RakeTask.new do |task|
  task.runner.configure do |dredd|
    dredd.paths_to_blueprints 'doc/*.apib'
  end
end

because the task exits early due to the # of arguments being checked before we attempt to run the server (which has the side effect of adding an argument).

This is just a spike to get things working, so feel free to recommend a better solution. I tried my best to keep the documentation and tests up to date.

Because we don't always need two arguments.
Instead uses the general String#argument_count helper.
Because we will eventually generate the second argument (api_endpoint)
when we start the rack server.
@rylnd
Copy link
Contributor Author

rylnd commented Aug 15, 2015

@gonzalo-bulnes the above scenario silently fails in both 0.5.0 and 0.6.0. It raises an InvalidCommandError in 0.7.0.

@gonzalo-bulnes
Copy link
Owner

@rylnd,

I think I made a mistake and added the validation too early when running the command (see the code affected by the bug, and the code I think should not be affected by the bug).

Would moving the command validation fix the issue?

# lib/dredd/rack/runner.rb

# ...

# Run Dredd
#
# Returns true if the Dredd exit status is zero, false instead.
def run
  start_server! unless api_remote?
  raise InvalidCommandError.new(command) unless command_valid?
  Kernel.system(command)
end

# ...

If it does, that would be simpler than adding specific command-length validation, don't you think?

@gonzalo-bulnes
Copy link
Owner

@rylnd Ok, so v0.6.0 is affected too, but fails silently (you were faster than I was). Performing the command validation after starting the server should solve both issues indeed, unless I'm wrong.

@rylnd
Copy link
Contributor Author

rylnd commented Aug 15, 2015

@gonzalo-bulnes yes, I agree. I saw that solution, and somehow convinced myself that we didn't want to start the server unless the command is valid. If the only consequence of the fix is a slightly longer wait before an InvalidCommandError is raised, that works for me.

@gonzalo-bulnes
Copy link
Owner

@rylnd Thanks, I have to leave right now, but I'll think about it, maybe starting the server is not that bad, but maybe you were right and we should improve the command validation. Quick failure is always a good feature to have!

@rylnd rylnd deleted the allow_single_blueprint_argument branch June 8, 2016 15:24
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.

None yet

2 participants