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

FeaturesLoader still supported? #733

Closed
willbryant opened this issue Sep 8, 2014 · 14 comments
Closed

FeaturesLoader still supported? #733

willbryant opened this issue Sep 8, 2014 · 14 comments

Comments

@willbryant
Copy link

The Cucumber::Runtime::FeaturesLoader class from 1.3 is still present in lib/cucumber/runtime/features_loader.rb, but it no longer works:

Exception when running features/admin_mep_proposal.feature: Cucumber::Ast no longer exists. These classes have moved into the Cucumber::Core::Ast namespace, but may not have the same API./Users/will/.rbenv/versions/2.0.0-p451/lib/ruby/gems/2.0.0/gems/cucumber-2.0.0.beta.2/lib/cucumber/ast.rb:10:in `const_missing'

This broke our cucumber parallelizer (nitra), so I started looking into what needs updating. But I notice that it's not actually used by the runtime class any more. Is it meant to still exist?

@mattwynne
Copy link
Member

No, we are planning to remove that class - we just haven't got around to clean up some of the dead code, sorry if that was confusing. The loading of features is mostly done by the cucumber-ruby-core now.

Can you show me the relevant code in nitra? Have you contacted the maintainers?

@willbryant
Copy link
Author

All good, now that I know that, I can remove it from nitra and invoke more of the cucumber code directly instead. Thanks.

@mattwynne
Copy link
Member

Are you the Nitra maintainer then?

Ping me in #cucumber if you need any help. We've not done any work to document the new APIs yet, so you'll have to read code or ask for help to figure it out.

Thanks for your understanding :)

@willbryant
Copy link
Author

Not officially, but I seem to be the only one patching it, so I guess so :)

It turned out to be easy - we just wanted to reset @loader so we could shove a new file into the funnel, so I now do that and then call the existing loader.

BTW, getting off-topic a bit, but would be nice to have a reset method like rspec has, to reset @results etc. ready for a new run.

All good for now though, thanks for the help.

@mattwynne
Copy link
Member

Can you give me an example / point me to some code about RSpec’s reset method?

@willbryant
Copy link
Author

Basically it resets the runtime's accumulated results and so on, so that it can be invoked with a new file - but without having to reload all the code such as step definitions, which would slow things down.

Here is what is monkeypatched in now:

module Cucumber
  module RbSupport
    class RbLanguage
      def load_code_file(code_file)
        require File.expand_path(code_file)
      end
    end
  end
  class ResetableRuntime < Runtime
    def reset
      @results = Results.new(@configuration)
      @loader = nil
    end
  end
end

@mattwynne
Copy link
Member

I love the comment at https://github.com/newporta/nitra/blob/master/lib/nitra/ext/cucumber.rb#L6! Amen.

Part of the motivation for 2.0 is to clear away the clutter so we can provide proper APIs to tools like Nitra.

Can we start from scratch and talk about what you'd ideally like the API to be? I see no reason why we can't release this with 2.0 if we can make some quick decisions.

@willbryant
Copy link
Author

I thought that comment was a bit harsh personally, as far as I can see they never even tried to get them upstreamed :).

The goal is to be able to reuse a runtime environment efficiently. The first method patch appears to be something to do with not reloading the step definitions unnecessarily (I'm just guessing that as it seems the only difference is changing load to require).

The second one resets the runtime so it is as if there have been no cukes run already - so it should do whatever is required to clear the logs and the list of scenarios run and so on, including making it so that the cukes already loaded will not be run again when run is called.

@os97673
Copy link
Member

os97673 commented Sep 16, 2014

@willbryant why do you need this functionality what problem you want/need to solve?

@willbryant
Copy link
Author

We hand out feature files one by one to the worker processes on the build farm. This results in much shorter build times than pre-partitioning the files into N groups because runtimes are always somewhat unpredictable.

So, we need to efficiently make the runtime run a newly-assigned file, without wasting time reloading all the step definitions and so on.

@mattwynne
Copy link
Member

Right, so ideally you want to be able to just do something like this:

# assuming runtime has already been constructed elsewhere...
next_feature do |feature|
  runtime.execute(feature)
end

Yeah?

@willbryant
Copy link
Author

Yeah :).

@mattwynne
Copy link
Member

OK @willbryant if you still care about this can you raise a new issue (or ideally a PR with a failing scenario that describes the API you'd like) then we can go from there?

@lock
Copy link

lock bot commented Oct 25, 2018

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@lock lock bot locked as resolved and limited conversation to collaborators Oct 25, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants