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

Fix handling of decision table with unknown output parameter #1178

Merged
merged 3 commits into from
Jan 18, 2019

Conversation

fhoeben
Copy link
Collaborator

@fhoeben fhoeben commented Jan 17, 2019

Fixes #1177

screenshot 2019-01-17 at 07 12 49

@fhoeben fhoeben merged commit 8f8f749 into unclebob:master Jan 18, 2019
@tcnh
Copy link
Contributor

tcnh commented Jan 29, 2019

There seems to be a side effect (possibly intentional?) to this change.
Prior to this change, we could define a scenario as follows:

| scenario     | my scenario    |
| action                        |
| $returnValue=| something else |

And call it as a DT like:

| my scenario  |
| returnValue? |
| expectation  |

This now throws the new exception. We now need to specify the return variable in the scenario signature. So to continue using our DT's we now need to refactor the scenario to a more verbose:

| scenario     | my scenario    | returnValue? |
| action                                       |
| $returnValue=| something else                |

Not a big issue, but might break some tests (for example BrowserTest.ScenarioTest in the HsacExamples).

@fhoeben
Copy link
Collaborator Author

fhoeben commented Jan 29, 2019

This is not intentional, but an expected side effect (as you may have noticed I did update some tests). Scenario's already required explicit input parameters and supported explicit output parameters. The fact that specifying the output was optional was unexpected and could lead to strange situations, as #1177.
The current check for whether an output parameter is present could possibly be improved, but erring on the side of expected behaviour and caution seemed wise in this case. Any suggestion (PR?) is more than welcome.

When using hsac-fitnesse-plugin: table template does not require explicit in- or output parameters. so in your case just replacing 'scenario' with 'table template' might be an easier fix than adding the explicit output parameter to the scenario's definition.

@maikelgithub
Copy link

We are experiencing serious issues since this change. All of our tests are now failing...

How does this work when having a scenario in a table template?

The goal of our tests is to fill a variable, so it's not part of any input.

All our tests now fail with the output:

Bad table! The argument yourvariable is not an output of the scenario.

@fhoeben
Copy link
Collaborator Author

fhoeben commented Apr 16, 2019

@maikelgithub can you give a sample wiki page with the scenario/table template combination that shows the issue you are experiencing?

@maikelgithub
Copy link

maikelgithub commented Apr 16, 2019

Hi @fhoeben,

thank you for your fast response.

We used to have similar code to this:

|scenario      |my division|numerator|and|denominator|
|setNumerator  |@numerator                           |
|setDenominator|@denominator                         |
|$myResult2=   |quotient                             |

|Library    |
|eg.Division|

|table template|test    |
|my division   |10|and|2|

|test       |
|my result2?|
|           |

This was still working on version 20190224.

With the new update, this is what I'm trying:

|scenario      |my division|numerator|and|denominator|makes|myResult2?|
|setNumerator  |@numerator                                            |
|setDenominator|@denominator                                          |
|$myResult2=   |quotient                                              |

|Library    |
|eg.Division|

|table template|test                     |
|my division   |10|and|2|makes|myResult2?|

|test      |
|myResult2?|
|          |

We're considering to keep working on version 20190224 for now. We have 100's of tests to adjust if we want to use the update of 20190409, while we never encountered issues with our scripts prior to the April update.

@fhoeben
Copy link
Collaborator Author

fhoeben commented Apr 16, 2019

I have not tried this combination. It works if the table template is converted to a normal scenario.
Can you file an issue on https://github.com/fhoeben/hsac-fitnesse-plugin, I believe the bug is in table template's code

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.

Unknown output column in decision table is ignored
3 participants