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

Symbols are not stored in the SlimTestContext #52

Closed
smithap opened this issue Feb 3, 2013 · 55 comments
Closed

Symbols are not stored in the SlimTestContext #52

smithap opened this issue Feb 3, 2013 · 55 comments

Comments

@smithap
Copy link

smithap commented Feb 3, 2013

If I create a symbol using "let", it's not available in another fixture, say a slim script fixture, as the variables are stored in a different place?

@smartrics
Copy link
Owner

They are currently not stored in the slim test context but in fitnesse
symbols map. Fitnesse#getSymbol

If I create a symbol using "let", it's not available in another fixture,
say a slim script fixture, as the variables are stored in a different place?


Reply to this email directly or view it on
GitHubhttps://github.com//issues/52.

@smithap
Copy link
Author

smithap commented Feb 5, 2013

Yes, not ideal. A lot of the standard Slim fixtures like the script one, have built in the ability to use the Test Context. If I want to pass variables back and forth, I end up having to write code to simply move them around. Wouldn't it be better if, when running in Slim mode, RestFixture used the same mechanism?

@smithap
Copy link
Author

smithap commented Feb 5, 2013

@smartrics
Copy link
Owner

I agree... But i won't implement it unless there's high demand for this feature.

On Feb 5, 2013 10:08 AM, "Adrian Smith" notifications@github.com wrote:

Yes, not ideal. A lot of the standard Slim fixtures like the script one,
have built in the ability to use the Test Context. If I want to pass
variables back and forth, I end up having to write code to simply move them
around. Wouldn't it be better if, when running in Slim mode, RestFixture
used the same mechanism?


Reply to this email directly or view it on GitHubhttps://github.com//issues/52#issuecomment-13122498.

@smartrics
Copy link
Owner

Please re-open if necessary. Unless there's demand for this feature I won't do it

@alvillaflor
Copy link

so what is the work-around to be able to use the variables in the rest fixture and make it available on slim fixtures? I too am trying to see if I can get the value.......
something like this? !define var {%restValue%}

@jplambert
Copy link
Contributor

Hi @smartrics

I am looking at the code, both RestFixture and FitNesse, and I am under the impression that the issue with symbols would be into FitNesse rather than into RestFixture.

What I am meaning: we should not use "let" to set values in variables but instead rely on Slim symbols as explained in http://www.fitnesse.org/FitNesse.UserGuide.WritingAcceptanceTests.SliM.SymbolsInTables

(let can still be useful as temp variables in the table to decompose a check of the extraction of a value)

But that does not work because FitNesse passes the symbols to the fixture, which looks to me as something dumb. FitNesse could replace referenced symbols with their values before passing the text to the fixture, and also remove any symbol assignment text. I'll give it a try but it looks like doing so would enable to use symbols in RestFixture (and any custom fixture, for that matter, including my hash fixture introduced in iisue #110).

I'll go even further, a quick look suggests a slightly off design there in FitNesse, because the other fixtures included in FitNesse appears to handle the symbol stuff all by themselves. It would make sense to factor this behavior (substituting referenced symbols with their value, assigning symbols) in the runner rather than in each fixture implementations. But maybe I'm wrong, I should have a closer look at all fixtures before claiming so...

I'll try hacking FitNesse to see if it enables the symbols into the custom fixtures. From there, I'll have to see if that actually make sense and talk about it on FitNesse's GitHub... :-)

What do you think?

@jplambert
Copy link
Contributor

Hey there, I got something working.

The following test works using the hack of FitNesse I suggeted: (DummyFixture simply returns the content of the first column in the second colum, as a way to perform tests oustide of RestFixture)

!define TEST_SYSTEM {slim}

!path dependencies/*
!path smartrics-RestFixture-3.2.jar
!path slf4j-simple-1.6.6.jar
!path DummyFixture.jar

|import|
|smartrics.rest.fitnesse.fixture|

| Table: Rest Fixture | http://localhost:8070 |
| GET | /TestRestFixture?rss | | | //title[text()='TestRestFixture']|
|let|id|js|response.body|$V=|

|Table: Dummy Fixture|
|sometext$V||

|Table: Dummy Fixture|
|root|$Y=|

| Table: Rest Fixture | http://localhost:8070 |
| GET | /$Y?rss | | | |

--> $V and $Y are assigned
--> $V and $Y are replaced by their values

ok1
ok2

Does that look right to you?

I'll bring the issue over FitNesse to see if what I changed makes sense. Maybe I misunderstood something in the design of FitNesse/Slim. In any case, this requires no change in RestFixture!

Thanks again.

@smartrics
Copy link
Owner

Hi - well done for the progress.

Preamble. Wherever possible the my approach has always been:

  • keep tests compatible between the slim and fitnesse runners. to the extent that it should only be necessary to swap the runner
  • wherever an incompatibility must be introduced then this should only be localised and side effects free.

With that in mind, I did look at refactoring smartrics.rest.fitnesse.fixture.support.Variables to handle either fit.FitSymbol or fitnesse.slim.VariableStore. that way the RestFixture encapsulates the two differences.
I never managed to start on the slim variant tho' so, I am not sure / I don't remember whether I proved that VariableStore is at the back of the slim symbols.
Obviously the drawback is that RestFixture would use one syntax that differs from the "standard" as described in SymbolsInTables.

So, Your proposal is more elegant even if it breaks the fit<->slim interop i was trying to achieve.
Anyway - I am ok with breaking the fit<->slim interop link if there's no other way to make RestFixture communicate with other fixtures and the FitNesse guys accept your solution.

Of the picture above am not sure why the output in grey is html formatted. is it supposed to be that way?

@smartrics smartrics reopened this Jan 29, 2015
@smartrics
Copy link
Owner

Hi - i have closed #110. Let's keep this one open as more pertinent to the problem we're trying to solve.

@jplambert
Copy link
Contributor

Yes in the picture the output in grey is HTML because it is the content of $V, which has been assigned the response body (HTML) of the first Rest call. FitNesse does a clean job in showing that: "$V <- [...]"

As for using VariableStore, I saw you started thinking about it with a VariableStore field, not used anywhere. But I don't see any way it could work... Unless I missed something, VariableStore is purely local. So using a VariableStore in RestFixture would maybe enable to share symbols between several RestFixture calls (as I believe you did some time ago with local 'let' variables) but not shared with other fixtures. I tried looking at how FitNesse and the Slim fixtures interacts, but I could not see how FitNesse would provide any variable store to the Slim fixtures, and I did not see either any static call that could be made from the Slim fixtures to get some shared variable store (which is good design, by the way). We could always do it using stack trace and reflexion of course, but I am pretty sure we don't want to go this way... ;-)

So basically RestFixture has currently no way to understand symbols defined by other fixtures, nor to define symbols for the other fixtures. Again, unless I'm wrong and missed something.

From there, my conclusion is that either the Slim Fixture contract needs to be changed to provide the variable stores to the Slim fixtures, or the symbols are meant to be handled by FitNesse/Slim server only. Which led to my proposal, by replacing the symbols at the Slim server level and removing the symbol assignment text provided to the fixture (since the fixture could not use this info anyway, not having access to the symbols).

I believe this may be what the FitNesse guys had in mind, since the symbol assignment stuff is already done in TableTable. It's a bit weird then that the symbol replacement was not there, and passing symbols to fixtures without access to the store of symbols do not make sense.

I'll share the link to the FitNesse issue I'll create about the subject and see how it goes.

Also, sorry to break the compatibility between Fit and Slim :-)

@smartrics
Copy link
Owner

Yeah as I remember the approach was to access the VariableStore instance
of the current execution context.

But I see what you are saying and even that approach requires a change to
fitnesse.

Well then over to you and the fitnesse team.
On 30 Jan 2015 07:27, "Jean-Pierre Lambert" notifications@github.com
wrote:

Yes in the picture the output in grey is HTML because it is the content of
$V, which has been assigned the response body (HTML) of the first Rest
call. FitNesse does a clean job in showing that: "$V <- [...]"

As for using VariableStore, I saw you started thinking about it with a
VariableStore field, not used anywhere. But I don't see any way it could
work... Unless I missed something, VariableStore is purely local. So using
a VariableStore in RestFixture would maybe enable to share symbols between
several RestFixture calls (as I believe you did some time ago with local
'let' variables) but not shared with other fixtures. I tried looking at how
FitNesse and the Slim fixtures interacts, but I could not see how FitNesse
would provide any variable store to the Slim fixtures, and I did not see
either any static call of other that could be made from the Slim fixtures
to get some shared variable store (which is good design, by the way). We
could always do it using stack trace and reflexion of course, but I am
pretty sure we don't want to go this way... ;-)

So basically RestFixture has currently no way to understand symbols
defined by other fixtures, nor to define symbols for the other fixtures.
Again, unless I'm wrong and missed something.

From there, my conclusion was that either the Slim Fixture contract needs
to be changed to provide the variable stores to the Slim fixtures, or the
symbols are meant to be handled by FitNesse/Slim server only. Which led to
my proposal, by replacing the symbols at the Slim server level and removing
the symbol assignment text provided to the fixture (since the fixture could
not use this info anyway, not having access to the symbols).

I believe this may be what the FitNesse guys had in mind, since the symbol
assignment stuff is already done in TableTable. It's a bit weird then that
the symbol replacement was not there, and passing symbols to fixtures
without access to the store of symbols do not make sense.

I'll share the link to the FitNesse issue I'll create about the subject
and see how it goes.

Also, sorry to break the compatibility between Fit and Slim :-)


Reply to this email directly or view it on GitHub
#52 (comment)
.

@jplambert
Copy link
Contributor

Hey, I resumed my work on the hash fixture, to stumble on some HTML code! You were right, something is wrong there. Sorry to dismiss your insightful comment!

Looks like I must do a slightly different code in FitNesse so that what is put in the variable is the text content, not the formatted content.

@jplambert
Copy link
Contributor

The request on FitNesse's GitHub: unclebob/fitnesse#604

@jplambert
Copy link
Contributor

About the HTML code that is stored in the variable, it seems it is added by RestFixture to provide a nice pretty-print. Of course when we want to set this in a symbol, that's not what we want.

Two options I see there:

  1. Add a configuration option that disables pretty printing
  2. Pass the assignment to RestFixture, so that it knows that the value will be assigned, and in that case disable pretty printing

So far I'm more for option 1. What do you think?

@jplambert
Copy link
Contributor

Actually, I am not sure if it makes sense to always disable pretty printing. The assignment case is only restricted to the "report" cell.

Right now I have been able to do what I wanted by hacking FitNesse and RestFixture. The question is: how to do it without hacking... I guess we'll have to solve how FitNesse handles slim symbols first, we'll see if something needs to be done with RestFixture after that.

@smartrics
Copy link
Owner

i agree - a property to disable pretty printing is too draconian.

Maybe tag of some sort to override the "do not pretty print" on a cell by cell basis.

| !- $V= -! |

@jplambert
Copy link
Contributor

After reading related issue in FitNesse's GitHub, it becomes clear that my solution is not OK. The problem is that I proposed handling the symbols at the level of TableTable, which processes the whole table result provided by RestFixture. That means that symbols asigned in a given row, won't be replaced in the following rows. So this must definitely be handled by RestFixture, with FitNesse giving access one way or another to the symbols store.

@smartrics
Copy link
Owner

notwithstanding it's a horrible hack, i wonder if you could do a Map Fixture that just copies the value.

|RestFixture| ...bla bla... |
| let | A | ...bla bla... |

|DT:export labels to slim symbols|
|rest fixture label| slim symbol? |
| A | $V= |

package smartrics.rest.fitnesse.fixture.support;
class ExportLabelsToSlimSymbols {
private String label;
public setRestFixtureLabel(String label)
this.label = label;
}

public slimSymbol() {
if (Fixture.hasSymbol(label)) {
return Fixture.getSymbol(label).toString();
}
return null;
}
}

@jplambert
Copy link
Contributor

That's an interesting suggestion!

However, following discussions on unclebob/fitnesse#525 I'm told there would be a way for RestFixture to store its symbols in the shared symbols table. So we could implement your grand vision of a common syntax between FIT and SLIM, with 'let' putting the symbols in the shared symbols table.

I'll check out if that's actually working and maybe I'll do something for RestFixture on this basis.

In any case I believe that would still not be perfect so we'll see if something is to be changed on FitNesse side anyway.

@alvillaflor
Copy link

I was going to say, that maybe that new fixture should be implemented as a fix for now. And if there is a better alternative that both restfixture and fitnesse could agree on, then use that but still make the map fixture as an alternative.

@jplambert
Copy link
Contributor

Hi alvillaflor,

If you are currently blocked and needing an urgent solution, I can share the hacks of FitNesse and RestFixture that I did. Using them, you'll be able to do '$V=' in the last column of 'let' rows.

Unfortunately I don't know how I can share the files...

JP

@alvillaflor
Copy link

Hi JP. not really blocked, just thinking that having 2 solutions as an end result is better than having none at all.

@smartrics
Copy link
Owner

@alvillaflor you can copy/paste the code above, fix it if necessary, and create the fixture yourself. It doesn't have to be in the RestFixture package for it to work.

@jplambert
Copy link
Contributor

@smartrics I have tested RestFixture implementing StatementExecutorConsumer, and it works: setStatementExecutor(StatementExecutorInterface) gets called on RestFixture, providing a hook on the runner environment.

However to make it work fully it needs "get" (there is only "assign" for now, on the latest FitNesse) so we'll have to add it in FitNesse before we can use it properly in RestFixture.

Just so you know, I have started refactoring RestFixture to include this: abstract the "Variables", and make it created by some factory depending on Slim/Fit runner type and providing access to the Slim execution environment (there is no static access, contrary to Fit Fixture environment). So don't bother to do it. I'll do a Pull Request about the subject. :-)

@smartrics
Copy link
Owner

Fantastic! well done and thanks a lot

On 3 February 2015 at 07:54, Jean-Pierre Lambert notifications@github.com
wrote:

@smartrics https://github.com/smartrics I have tested RestFixture
implementing StatementExecutorConsumer, and it works:
setStatementExecutor(StatementExecutorInterface) gets called on
RestFixture, providing a hook on the runner environment.

However to make it work fully it needs "get" (there is only "assign" for
now, on the latest FitNesse) so we'll have to add it in FitNesse before we
can use it properly in RestFixture.

Just so you know, I have started refactoring RestFixture to include this:
abstract the "Variables", and make it created by some factory depending on
Slim/Fit runner type and providing access to the Slim execution environment
(there is no static access, contrary to Fit Fixture environment). So don't
bother to do it. I'll do a Pull Request about the subject. :-)


Reply to this email directly or view it on GitHub
#52 (comment)
.

Regards,

Fabrizio

@darthwangusa
Copy link

great! that is good news. Hopefully, it does not take very long.

Would that change be the same as what smartrics mentioned above? Or is there something I can use now to use with slim instead of fit to make it work like the table below?

|DT:export labels to slim symbols|
|rest fixture label| slim symbol? |
| A | $V= |

@jplambert
Copy link
Contributor

@darthwangusa,

I took a second look at smartrics's last snipper, and I don't believe it would work. Because that would store values in Fixture, which is only used in FIT. Using SLIM, things are much less straightforward to access to the table of symbols, and is currently not implemented in RestFixture.

With the changes I will propose in a pull request, RestFixture will have proper access to the table of symbols, and thus will be able to assign values into the pool of variables shared with other fixtures. Currently this is not the case: variables assigned in RestFixture are in a local store and can be used in following RestFixture calls, but not in other fixtures.

However to have the fix complete, RestFixture should not only be able to assign values in the table of symbols, but also to read values from it. This is not possible at all currently, and will be the subject of the pull request I will propose to the FitNesse guys. That way, using RestFixture will work the same in both FIT and SLIM.

If you are stuck and need a quick and dirty work around, I can share my small patches that make FitNesse handle it instead of RestFixture, that I mentioned in my previous message. With it I can write this kind of thing in SLIM:

|Table: Rest Fixture|http://localhost|
|GET|/ws/getid|200|Content-Type : application/json||
|let||js|JSON.parse(response.body).stuff[0].id|$id=|

|Table: Hash Fixture|SHA-256|
|$idairing|$hash=|

|Table: Rest Fixture|http://localhost|
|GET|/ws/dostuff?hash=$hash&id=$id|200|Content-Type : application/xml; charset=UTF-8|'![CDATA[OK]]'|

@darthwangusa
Copy link

cool. that would be good. thanks

@jplambert
Copy link
Contributor

Just to tease you, using my local changes to FitNesse and RestFixture it seems to work as expected:

withpullrequests

(not shown in the screenshot, we are using SLIM here)

Please note, that using "$id=" in the last cell of the 'let' won't work, the same as it does today, because RestFixture will try to match the text. We could implement some change there to detect this case properly, and set the table of symbol in that case. (not only on 'let' variables)

I'll make the FitNesse pull request as soon as possible.

@darthwangusa
Copy link

thinking about that. if you do it that way.
does it mean that if it is '$id=' that value is assigned to id; while '$id', the value will be asserted against the value of $id?

@jplambert
Copy link
Contributor

Yes that's how Slim symbols are supposed to work. You can have a look at FitNesse's documentation on Slim symbols.

At least, that's how it works for instance on Decision Tables. I guess it would make sense to have the same behavior on TableTable custom fixtures, even though in practice it is entirely up to the implementor of the fixture. By the way, you can see that FitNesse will format the result as "$V<-[value]" even if the underlying fixture did not assign anything to $V.

@jplambert
Copy link
Contributor

I'll make the pull request ASAP so that people can work around the issue using the branches of FitNesse and RestFixture pull requests. Better than to share dirty patches.

@jplambert
Copy link
Contributor

Hi, you'll find the pull request here: #114

If you want to try it out, you'll need my other pull request about FitNesse: unclebob/fitnesse#644

I hope my pull request is fine, I had some troubles with my early GitHub setup, lost some code and had to rewrite it.

@smartrics
Copy link
Owner

I will try and merge this pull request in a branch of the RestFixture and merge only when the FitNesse patch is successfully pulled.

@jplambert
Copy link
Contributor

Thanks. I hope I did not twist the existing code!

@darthwangusa
Copy link

Hi JP. I am searching around the web and found this implementation from 'Gregor Gramlich'. How different is his implementation from yours?

url: https://groups.yahoo.com/neo/groups/fitnesse/conversations/topics/20571
Hi,

I started a branch of Rest Fixture two years ago, that contains a new subclass RestScriptFixture of RestFixture.
https://github.com/ggramlich/RestFixture/commits/ScriptTableRestFixture

It allows to use the Fixture in a normal Slim Script table and thus also for Scenarios and the common way of Slim Symbol usage.
http://fitnesse.org/FitNesse.UserGuide.SliM.SymbolsInTables

The usage can be seen especially in this commit:
ggramlich@07eabc5

Here's some more explanation of the intent
https://github.com/ggramlich/RestFixture/blob/07eabc5b61a0a1011a5292696f9227af9c3be938/src/test/cat/FitNesseRoot/SlimScriptTests/content.txt

I do not know, how easy it is to merge with Fabrizio's current master, but maybe someone of you finds it useful and wants to give it a try.

Gregor

@jplambert
Copy link
Contributor

:-D thanks for the compliment darthwangusa, but RestFixture is the work of smartrics! Actually I am an all new user and very minor contributor of RestFixture :-)

I'll let smartrics answer to your question, but I want to add this:

The project which we are testing is actually made in PHP, and as much RestFixture has been useful early on to play with the Rest API, we are now moving forward to PHP-only fixtures since we need to master carefully the data in database before making the Rest API calls, and since we can't mix PHP and Java fixtures in the same FitNesse test run.

So we are thinking seriously about making a PHP alternative to RestFixture, though it will probably be less advanced while providing other features specific to our needs (like checking that a given JSON response body matches a specific JSON schema).

However I don't think we'll use the same syntax than RestFixture. I think we'd rather make a ScriptTable fixture, which look like it would do the job very well for less work than making a TableTable fixture. I understand this is what proposed Gregor Hamlich.

Indeed following the ScriptTable fixture adds the convenience of Slim symbols working already, and the ability to make scenario out of them to be used in other tests.

@jplambert
Copy link
Contributor

Hey @smartrics, have you seen that my Pull Request have been merged in FitNesse? I'm not sure to understand when they do release new versions, but basically that means we can work on my Pull Request for RestFixture as soons as a new release is available.

@eamonnfaherty-productmadness

Thanks for your work on this @jplambert

@smartrics
Copy link
Owner

JP - thanks for the heads up... I will keep an eye on the next release.
BTW i am thinking of changing the release version to 4.0 given that it will break backward compatibility with old versions of fitnesse

@jplambert
Copy link
Contributor

Thanks guys,

Good idea @smartrics to chgange the version number. You can also explain somewhere that if people get some MethodNotFoundException about the StatementExecutorInterface#getSymbol(String) method then it is certainly because they upgraded RestFixture but not FitNesse.

Please let me know how the merge of my PR with RestFixture goes, do not hesitate to ask help or to comment anything about the PR.

@smartrics
Copy link
Owner

Closing this issue - fix available in v 4.0
Acceptance tests available in RestFixtureLiveDoc (LetTest)

@smartrics
Copy link
Owner

JP, Thanks for the contribution!

@jplambert
Copy link
Contributor

Thanks @smartrics I did it gladly.

However I just noticed that I forgot an old 'TODO' in the code that should be removed not to induce into error future maintainers. I put a review comment on SlimVariables to point it out, can you see it?

@smartrics
Copy link
Owner

OK - np - I'd appreciate if you patch the master branch and send a patch or a new pull request. i assume it's not going to require a new release?

@jplambert
Copy link
Contributor

HI @smartrics just made the Pull Request #119 for this minor change.

Indeed it does not require any new release, as it is just about removing an unhelpful comment.

I have a question however about the releases. I believe before release 4.0, RestFixture was at 3.2. Still I can't see any 3.2 release in https://github.com/smartrics/RestFixture/releases in case people do not want to move to 4.0 (yet). That was exactly what we had at work: release 4.0 broke the setup script but we had not easy way to stay on 3.2 :-)

@smartrics
Copy link
Owner

Thanks!

I have never released 3.2 because it became 4.0 with your pull request. Although I had the pom version to 3.2 for a while - so you must have had a version that you built from master branch

@darthwangusa
Copy link

Hi. I pulled both the 4.0 version and the latest fitnesse version that would support the changes and now the 'let' call in the script is failing.

Also JP. can you please provide a small snippet here on how to use that change?
thanks.

@lborie
Copy link

lborie commented Jul 28, 2015

I have a question, what about if there is some "normal %" in my URL ? Like in an encoded url ?

| Table:smartrics.rest.fitnesse.fixture.RestFixture | ${MY_SERVER} |
| setHeader | Content-type: text/plain | |
| GET | /myapi?redirect_uri=http%3A%2F%2Flocalhost%3A8080 | 200 | | |

This generates a NullPointerException

Caused by: java.lang.NullPointerException at smartrics.rest.fitnesse.fixture.support.SlimVariables.get(SlimVariables.java:64)

@smartrics
Copy link
Owner

Ok thanks for reporting

I will fix it
On 28 Jul 2015 16:19, "Ludovic Borie" notifications@github.com wrote:

I have a question, what about if there is some "normal %" in my URL ? Like
in an encoded url ?

| Table:smartrics.rest.fitnesse.fixture.RestFixture | ${MY_SERVER} |
| setHeader | Content-type: text/plain | |
| GET | /myapi?redirect_uri=http%3A%2F%2Flocalhost%3A8080 | 200 | | |

This generates a NullPointerException

Caused by: java.lang.NullPointerException
at
smartrics.rest.fitnesse.fixture.support.SlimVariables.get(SlimVariables.java:64)


Reply to this email directly or view it on GitHub
#52 (comment)
.

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

No branches or pull requests

7 participants