Allow test blocks to access private members of objects

[eladb]

When writing tests it's very common to need to inspect the internal state of an object.

In most programming languages, it is a huge hassle because internal state is usually protected by the type system though access modifiers (private or protected). Developers work around this by making these APIs public, extracting them to separate functions or other creative means.

Since Wing has testing as first-class citizen, we can decide that code written within a test block in Wing will have access to private members as if they were marked as internal (i.e. within the same library).

And the world will be a better place.

[ShaiBer]

I like it! And don't see a reason not to support it

[skyrpex]

I feel like this would open the door to unhealthy tests, unless the test is created within the class itself of course.

// This makes sense! Any inflight declared inside a class (including tests) should have access to their private scope.
class Foo {
  new() {
    test "foo" {
      assert(this.privateFoo() == "foo");
    }
  }
  inflight privateFoo() {
    return "foo";
  }
}

// This doesn't make sense IMO.
class Bar {
  inflight privateBar() {
    return "bar";
  }
}

let bar = new Bar();
test "bar" {
  assert(bar.privateBar() == "bar");
}

[olivernybroe]

This is actually an interesting solution. Essentially doing scoped tests.

So scoping a test inside the class gives it access to the class scope.

[MarkMcCulloh]

Something like this feels better to me, and ideally, it'd be nice if we could figure out a way to have this as a class-level declaration.

class Foo {
  inflight privateFoo() {
    return "foo";
  }
  preflight test {
    let f = new Foo();
    test "foo" {
      assert(this.privateFoo() == "foo");
    }
  }
}

This is also in line with the general goal to allow test statements in non-entrypoints as well.

[olivernybroe]

Two immediate problems I see with this approach:

• You cannot group all your tests in a test file or at the end of your file.
• You cannot access private properties from multiple classes in the same test. (might be a fine limitation)

[MarkMcCulloh]

You cannot group all your tests in a test file or at the end of your file.

Depending on how one uses files, there may not be a distinction here (i.e. 1 file = 1 class). I think this is ok because the grouping here is tied to what your testing rather than a more arbitrary grouping at the end of the file.

You cannot access private properties from multiple classes in the same test. (might be a fine limitation)

I think, in general, the more limited this is, the better. Most people here are saying that testing internals is a smell and I'm not sure I agree with that entirely, but I would definitely say that testing multiple separate internals is a smell.

Consider a class as just a namespace. Like a file namespace, it would make sense that a test declared in that namespace can access private symbols. As a unit of encapsulation, I'm not sure I see why classes should be special here.

[yoav-steinberg]

And the world will be a better place.

🤔 Except this encourages not testing the contract, and testing internal implementation, which is generally not what you want.

I'm on the fence here.

[skyrpex]

Exactly. See my comment here #5802 (comment).

[yoav-steinberg]

But I suspect that our conservative approach here is purely theoretical. It might happen that relaxing this limitation would lead to more tests, less bugs and will improve the software overall. I suspect this isn't usually supported not because of any empirical evidence but more because of a purist approach. I'm leaning towards relaxing this.

[ekeren]

Testing private API is considered a smell, but it is not uncommon to do.

As a reference Android development has @visibleForTesting:

@VisibleForTesting(otherwise = VisibleForTesting.PROTECTED)
public String printDiagnostics() { ... }

I like seeing this smell in the code, it might indicate that I need to extract an internal class.

If we had annotation, I would love something that would indicate the fact that this method is tested.

[skyrpex]

There's no need to relax this constraint if you can create the tests inside the class' private context :)

[ekeren]

It will make it impossible to write multi-platform tests.

Right now tests are multi clouds (multi platforms), when we add a test that tests internal implementation, we put a weird constraints on someone that will want to extend the API for another platform.

Think about cloud.API which was implemented by three different people (sim, tf-aws, awscdk), if someone would have added a private accessing test it would mandate the internal platform implementation of that class

[yoav-steinberg]

I'm not sure about this. Our dep-injection mechanism doesn't expose the implementation class at all. The compiler will complain that the field is unknown because all it knows about is the base class for all platforms (the abstraction). The platform specific implementations need their own tests.

[ekeren]

The compiler will complain that the field is unknown because all it knows about is the base class for all platforms

Makes sense, thanks

[olivernybroe]

Just to suggest another alternative of this with a form of scoping keyword.

This keyword allows you to say that the test is scoped to this class, given it access to private values in that class.

class Bar {
  inflight privateBar() {
    return "bar";
  }
}

test "bar" with Bar {
   let bar = new Bar();
   assert(bar.privateBar() == "bar");
}

The syntax could support multiple values even

test "bar and foo" with Bar, Foo {
   let bar = new Bar();
   assert(bar.privateBar() == "bar");
}

Would make it a very active choice in the test if it should be allowed to touch private properties.

[marciocadev]

Perhaps we could create an "expose" marker in the tests indicating which private method the test can access.

Something like this

class Bar {
  inflight privateBar() {
    return "bar";
  }
}

let bar = new Bar();
test "bar" {
  assert(bar.privateBar() == "bar");
} expose "privateBar";

Or perhaps use "expose" to indicate the class and expose all methods, but the important thing would be to indicate this exposure for the test.

class Bar {
  inflight privateBar() {
    return "bar";
  }
}

let bar = new Bar();
test "bar" {
  assert(bar.privateBar() == "bar");
} expose "Bar";

[eladb]

How about we allow a test to access private members only if it's defined in the same file as the class.

(Basically treat it as internal)

[marciocadev]

I'm a little hesitant about that, Elad. In large projects, keeping tests alongside the code to be tested can create difficulty in readability and navigation within the code.