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

where block cannot access or declare variables? #138

Open
robfletcher opened this issue Aug 29, 2015 · 22 comments
Open

where block cannot access or declare variables? #138

robfletcher opened this issue Aug 29, 2015 · 22 comments

Comments

@robfletcher
Copy link
Contributor

Originally reported on Google Code with ID 15

i would expect to be able to do this:

def "my test"() {
  given:
      def a = new Apple()
      def o = new Orange()

  when:
      fruit = f
  then: 
      <something>
  where:
       f << [a, o, a, o, a, o]  
}

But I get MissingPropertyException for a & o.  I also tried declaring a & o in the
where block - same issue.  Declaring them outside the 
scope of the method works but is not ideal.  I ended up unrolling the where into several
when/then blocks.

I see that the documentation for the where block is coming (at light speed :) so maybe
this is as designed.

Thanks

Reported by jacobsonz on 2009-03-12 18:07:08

@robfletcher
Copy link
Contributor Author

In your concrete example, you could do:

where:
f << [new Apple(), new Orange(), ...]

You could also call out to factory methods like this:

f << [createApple(), createOrange(), ...]

Note that such factory methods may only access @Shared and static fields of the
specification class (but no instance fields).

Is any of these options a good solution for your use case, or would you prefer to do
something like the following?

where:
def a = new Apple()
def b = new Orange()
f << [a, o, a, o, a, o]


Reported by pniederw on 2009-03-12 18:44:27

  • Labels added: Type-Enhancement
  • Labels removed: Type-Defect

@robfletcher
Copy link
Contributor Author

Reported by pniederw on 2009-03-12 18:44:43

@robfletcher
Copy link
Contributor Author

I think the last option is the best because the actual test we were writing was more
like:
  f << [[a, o, a], [a, a, a], [a,o], [o,o,o], [a]]

and it would be silly to keep instantiating.

Reported by jacobsonz on 2009-03-12 22:32:25

@robfletcher
Copy link
Contributor Author

I understand your point. On the other hand, encouraging the reuse of objects in such
a way goes somewhat 
against the current design, which tries to isolate iterations as much as possible from
each other. For example, 
setup()/cleanup() is called before/after every iteration. And, unless your objects
are heavy-weight, recreating 
them is no significant overhead. I will have to think about this.

Currently, you can achieve your goal as follows:

@Speck
class MySpeck {
  @Shared a = new Apple()
  @Shared o = new Orange()

  def "my test"() {
    // ...

    where:
    f << [[a, o, a], [a, a, a], [a,o], [o,o,o], [a]]
  }
}

The downside is that a and o are in a sense defined in the wrong scope and could be
used by other feature 
methods as well.


Reported by pniederw on 2009-03-12 23:11:19

@robfletcher
Copy link
Contributor Author

Here is another way:

where:
f << {
  def a = new Apple()
  def o = new Orange()
  [[a, o, a], [a, a, a], [a,o], [o,o,o], [a]]
}()

Reported by pniederw on 2009-03-19 21:12:54

@robfletcher
Copy link
Contributor Author

That's a nice solution.  I like it.  Thanks!

Reported by jacobsonz on 2009-03-19 23:25:44

@robfletcher
Copy link
Contributor Author

For now I've decided not to add support for variable definitions in where-blocks, unless
a compelling use-case 
comes along.

Reported by pniederw on 2009-03-30 01:12:29

  • Status changed: WontFix

@craffael
Copy link

craffael commented Aug 7, 2017

It has happened to me multiple times now that I've run into a similar issue and I would very much appreciate if it would at least be possible to access instance variables of the specification from the where block. Consider for example the following test case (which doesn't compile):

class NotSupportedSpec extends Specification {

  Consumer<Integer> userDeleteListener = Mock()
  def userManager = new UserManager()

  def userId1 = userManager.addUser("romeo", "street 1", "+13212342")
  def userId2 = userManager.addUser("julia", "street 2", "+43212322")

  def setup() {
    userManager.addDeleteListener(userDeleteListener)
  }

  def deleteTest() {
    when:
      userManager.deleteUser(userId)
    then:
      1 * userDeleteListener.accept(_)
      !userManager.userExists(userId)
    
    where:
      userId  | _
      userId1 | _
      userId2 | _
  }

  // more tests that build on userId1, userId2 and userDeleteListener (possibly with where blocks)
}

Note that I cannot annotate userManager with @shared because then I could not register the listener anymore (which is a mock object). I could of course unroll the deleteTest but I don't think that this is satisfactory. Also it would of course be possible to register the listener in the test and annotate the userManager with @shared. But assume that some other methods must be called in setup() after the listener has been registered...

@tjni
Copy link

tjni commented May 25, 2018

I was just bitten by this as well, so please use as another datapoint. I was roughly trying to do the following:

where:
def sep = File.separator

path              | startsWithPath || result
""                | ""             || true
"a${sep}a${sep}a" | "a${sep}b"     || false
"a${sep}a"        | "a${sep}a"     || true
"a${sep}a${sep}a" | "a${sep}a"     || true
"a${sep}ab"       | "a${sep}a"     || false

@FatCash
Copy link

FatCash commented Dec 18, 2018

+1 for this feature!

2 similar comments
@nutscrack
Copy link

+1 for this feature!

@otaviomedeiros
Copy link

+1 for this feature!

@Vampire
Copy link
Member

Vampire commented Sep 13, 2019

@craffael If generally making these fields @Shared would work for you, you could make them shared, and instead of having a setup method have a method called differently and and call that helper method from inside your test. Or if the listener could also be shared, you could use setupSpec instead of setup. Or you could define userManager, userId1 and userId2 inside the where block and then have a helper method with userManager as parameter that prepares the user manager as you need it.

@tjni Just remove the def and it works.
The minor downside (or upside, depends on point of view) is, that sep will then also be available in the actual test code.
You can define calculated data variables, even calculating the value from previously defined data variables, just not local variables that are only available in the where block.

@Vampire
Copy link
Member

Vampire commented Sep 13, 2019

@craffael actually I had a similar problem right now and found an even groovier solution.
Use the dynamic field resolution of Groovy.
Here your code modified to clarify what I mean:

class SupportedSpec extends Specification {

  Consumer<Integer> userDeleteListener = Mock()
  def userManager = new UserManager()

  def userId1 = userManager.addUser("romeo", "street 1", "+13212342")
  def userId2 = userManager.addUser("julia", "street 2", "+43212322")

  def setup() {
    userManager.addDeleteListener(userDeleteListener)
  }

  def deleteTest() {
    when:
      userManager.deleteUser(this."$userId")
    then:
      1 * userDeleteListener.accept(_)
      !userManager.userExists(this."$userId")
    
    where:
      userId    | _
      'userId1' | _
      'userId2' | _
  }

  // more tests that build on userId1, userId2 and userDeleteListener (possibly with where blocks)
}

@Vampire
Copy link
Member

Vampire commented Sep 13, 2019

Or if you like it better:

class SupportedSpec extends Specification {

  Consumer<Integer> userDeleteListener = Mock()
  def userManager = new UserManager()

  def userId1 = userManager.addUser("romeo", "street 1", "+13212342")
  def userId2 = userManager.addUser("julia", "street 2", "+43212322")

  def setup() {
    userManager.addDeleteListener(userDeleteListener)
  }

  def deleteTest() {
    given:
      userId = this."$userId"
    when:
      userManager.deleteUser(userId)
    then:
      1 * userDeleteListener.accept(_)
      !userManager.userExists(userId)
    
    where:
      userId    | _
      'userId1' | _
      'userId2' | _
  }

  // more tests that build on userId1, userId2 and userDeleteListener (possibly with where blocks)
}

@craffael
Copy link

@Vampire Thanks for the tip with dynamic field resolution, that's actually a workable workaround :)

@chochos
Copy link

chochos commented Dec 7, 2021

this really hurts readability. I have a test where I want to test some date ranges. I would have liked to do this:

  //put this anywhere in the spec
  Date longAgo = new Date(System.currentTimeMillis() - 86_400_000_000L)
  Date moreRecent = new Date(System.currentTimeMillis() - 86_400_000L)
  Date now = new Date()
where:
  p1 | p2 | since      | until      || expect
  v1 | v2 | longAgo    | now        || foo
  v2 | v3 | longAgo    | moreRecent || bar
  v3 | v4 | moreRecent | now        || baz

But I have to include those long instantiations everywhere, or create methods that return them.

@leonard84
Copy link
Member

@chochos you can access @Shared/static fields in where: blocks https://spockframework.org/spock/docs/2.0/all_in_one.html#_sharing_of_objects_between_iterations

@wisnij
Copy link

wisnij commented Aug 16, 2022

@Vampire can you expand on your reply to #138 (comment) above? You said "Just remove the def and it works", but I tried modifying @tjni's example as follows:

where:
sep = "."

path              | startsWithPath || result
""                | ""             || true
"a${sep}a${sep}a" | "a${sep}b"     || false
"a${sep}a"        | "a${sep}a"     || true
"a${sep}a${sep}a" | "a${sep}a"     || true
"a${sep}ab"       | "a${sep}a"     || false

and in 2.1 it gave me groovy.lang.MissingPropertyException: No such property: sep for class: MyTestClass. (In 1.3 I got java.lang.NullPointerException: Cannot invoke method getAt() on null object instead, at the same line number.)

I'm trying to something similar in my own code, e.g.:

expected = 'Expected'
maps << [
  [[VAR: expected]],
  [[OTHER: 'Unexpected', VAR: expected]],
  [[:], [VAR: expected]],
  [[VAR: expected], [:]],
  [[VAR: expected], [VAR: 'Unexpected']],
]

and I'm also getting that error as well.

@Vampire
Copy link
Member

Vampire commented Mar 28, 2023

Sorry for the late answer @wisnij.
I think in 1.3 it should have worked iirc, but since 2.0 it will not anymore due to various quirks and problems it caused and various cases that did not work anyway as you can read in the 2.0 migration guide: https://spockframework.org/spock/docs/2.3/all_in_one.html#_no_access_to_data_variables_in_data_pipes_anymore_2

As @leonard84 mentioned, you can declare expected as @Shared or static (if it is a constant) field and then access it in the where block.

@leonard84 maybe now that we forbid to access other data variables it might be time to actually reopen this issue and finally allow local variables in the where block? What do you think?

@leonard84
Copy link
Member

@Vampire if you want to tackle that, you can give it a try.
I think that they would basically behave like scoped constants that are available only to the where block. Furthermore, to make it less confusing I would say you can only declare them as first expressions of a where block. Maybe we should require final instead of def to make it clear that they won't be reassigned.

where:
final sep = "."

path              | startsWithPath || result
""                | ""             || true
"a${sep}a${sep}a" | "a${sep}b"     || false
"a${sep}a"        | "a${sep}a"     || true

The following would be illegal:

where:
a | b
1 | 2

final c = a + b
d << [c, c]

@Vampire Vampire reopened this Apr 3, 2023
@RobynLiu
Copy link

RobynLiu commented Sep 1, 2023

+1

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests