Details
Description
In multi-threaded mode, the steps instances need to be instantiated per thread.
A way to do this is to pass the InjectableStepsFactory to the StoryRunner run context and let the context instantiate the candidate steps per thread.
Activity
Mauro Talevi
made changes -
Field | Original Value | New Value |
---|---|---|
Summary | ConfigurableSteps.addSteps(..) should take classes not instances. | ConfigurableEmbedder.addSteps(..) should take classes not instances. |
Issue Type | Bug [ 1 ] | New Feature [ 2 ] |
Fix Version/s | 3.x [ 16979 ] |
Paul Hammant
made changes -
Description |
public void addSteps(List<Class> steps) {
} ... would be better. Why? There's an instance that's made during the setup of a suite (refer EtsyStories). If you are running in multi-threaded mode, that instance is **never** used during the running of scenarios. In multi-threaded mode, there's a constructor for BasePage that takes a WebDriverProvider. When it is passed in during the primordial instantiation phase, no WebDriver has been initialized. We don't want to do that of course because (a) we know this instance is going to be garbage collected and never used, and (b) on SauceLabs at least it would result in browser opening in the cloud then closing (or maybe not closing, but never used; I'm not sure). JBehave does not actually need the instance. I believe this is provable. Instead it could perfectly well use the class definition without the instance ref somehow. Proposal: Long overdue, I think, but could for 4.x we shift to using class-defs. Either rework CandidateSteps to hold a Class rather than an instance. Or further do a away (move the logic elsewhere) with CandidateSteps, and use the POJO Step's class-def directly. |
public void addSteps(List<Class> steps) {
} ... would be better. Why? There's an instance that's made during the setup of a suite (refer EtsyStories). If you are running in multi-threaded mode, that **initial** instance is **never** used during the running of scenarios. In multi-threaded mode, there's a constructor for BasePage that takes a WebDriverProvider. When it is passed in during the primordial instantiation phase, no WebDriver has been initialized. We don't want to do that of course because (a) we know this **initial** instance is going to be garbage collected and never used, and (b) on SauceLabs at least it would result in browser opening in the cloud then closing (or maybe not closing, but never used; I'm not sure). JBehave does not actually need the **initial** instance. I believe this is provable. Instead it could perfectly well use the class definition without the instance ref somehow. Proposal: Long overdue, I think, but could for 4.x we shift to using class-defs. Either rework CandidateSteps to hold a Class rather than an instance. Or further do a away (move the logic elsewhere) with CandidateSteps, and use the POJO Step's class-def directly. |
Mauro Talevi
made changes -
Summary | ConfigurableEmbedder.addSteps(..) should take classes not instances. | CandidateSteps instances should be created by StoryRunner context allowing for multi-threaded stateful steps logic |
Mauro Talevi
made changes -
Assignee | Mauro Talevi [ maurotalevi ] | |
Fix Version/s | 3.4 [ 17278 ] | |
Fix Version/s | 3.x [ 16979 ] |
Mauro Talevi
made changes -
Description |
public void addSteps(List<Class> steps) {
} ... would be better. Why? There's an instance that's made during the setup of a suite (refer EtsyStories). If you are running in multi-threaded mode, that **initial** instance is **never** used during the running of scenarios. In multi-threaded mode, there's a constructor for BasePage that takes a WebDriverProvider. When it is passed in during the primordial instantiation phase, no WebDriver has been initialized. We don't want to do that of course because (a) we know this **initial** instance is going to be garbage collected and never used, and (b) on SauceLabs at least it would result in browser opening in the cloud then closing (or maybe not closing, but never used; I'm not sure). JBehave does not actually need the **initial** instance. I believe this is provable. Instead it could perfectly well use the class definition without the instance ref somehow. Proposal: Long overdue, I think, but could for 4.x we shift to using class-defs. Either rework CandidateSteps to hold a Class rather than an instance. Or further do a away (move the logic elsewhere) with CandidateSteps, and use the POJO Step's class-def directly. |
In multi-threaded mode, the steps instances need to be instantiated per thread.
A way to do this is to pass the InjectableStepsFactory to the StoryRunner run context and let the context instantiate the candidate steps per thread. |
Mauro Talevi
made changes -
Status | Open [ 1 ] | In Progress [ 3 ] |
Mauro Talevi
made changes -
Status | In Progress [ 3 ] | Resolved [ 5 ] |
Resolution | Fixed [ 1 ] |
End-users. In the meantime, if you're trying to do the WebDriver PageFactory style of field initialization in the constructor to BasePage (refer http://code.google.com/p/selenium/wiki/PageFactory), change from this style:
PageFactory.initElements(driverProvider.get(), this);
.. to this style:
try
{ PageFactory.initElements(driverProvider.get(), this); }catch (Exception e)
{ // this is OK, the first instance of each page is created by JBehave for the purposes of inspecting the methods of steps classes. The instance itself is not needed. Therefore other methods in this class are not called for an instance that has skipped the WebDriver PageFactory style field initialization. }