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

Unable to create test for non-empty constraint on listing #461

Open
jsundin opened this issue Apr 28, 2024 · 2 comments
Open

Unable to create test for non-empty constraint on listing #461

jsundin opened this issue Apr 28, 2024 · 2 comments

Comments

@jsundin
Copy link

jsundin commented Apr 28, 2024

The intention here is to create a test for checking a non-empty constraint on a listing, for regression purposes. I realize that pkl test has not been documented, so it's quite possible there are some misunderstandings or unsupported functionality being used here.

We need three files for a demonstration.

# NonEmptyDef.pkl:
values: Listing<String>(!isEmpty)

# NonEmptyImpl.pkl:
amends "NonEmptyDef.pkl"
 
values {
}

# NonEmptyTest.pkl:
amends "pkl:test"
import  "pkl:test"
import "NonEmptyImpl.pkl" as confUnderTest
 
examples {
    ["basic-test"] {
        test.catch(() -> confUnderTest)
    }
}

We can clearly see that values defined in NonEmptyImpl is invalid, as the listing is required to not be empty. However, when running this test:

$ pkl test --overwrite NonEmptyTest.pkl 
module NonEmptyTest (file:///tmp/issues/NonEmptyTest.pkl, line 1)
  basic-test ✍️
  NonEmptyTest ❌
    Error:
        -- Pkl Error --
        Expected an exception, but none was thrown.
        
        7 | test.catch(() -> confUnderTest)
            ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
        at NonEmptyTest#examples["basic-test"][#1] (file:///tmp/issues/NonEmptyTest.pkl, line 7)

The error seems to suggest that there is no evaluation error here. If that's the case, we should be able to try the inverse test, as demonstrated in NonEmptyTest-Inv.pkl:

amends "pkl:test"
import  "pkl:test"
import "NonEmptyImpl.pkl" as confUnderTest
 
examples {
    ["basic-test"] {
        confUnderTest
    }
}

This time we get the opposite error instead; an error was thrown and we did not catch it:

$ pkl test --overwrite NonEmptyTest-Inv.pkl 
module NonEmptyTest-Inv (file:///tmp/issues/NonEmptyTest-Inv.pkl, line 1)
  basic-test ✍️
  NonEmptyTest-Inv ❌
    Error:
        -- Pkl Error --
        Type constraint `!isEmpty` violated.
        Value: new Listing {}
        
        1 | values: Listing<String>(!isEmpty)
                                    ^^^^^^^^
        at NonEmptyDef#values (file:///tmp/issues/NonEmptyDef.pkl, line 1)
        
        3 | values {
            ^^^^^^^^
        at NonEmptyImpl#values (file:///tmp/issues/NonEmptyImpl.pkl, line 3)

If we instead embed this list inside a mapping, this works as expected:

# NonEmptyDef2.pkl:
stuff: Mapping<String, Listing<String>(!isEmpty)>

# NonEmptyImpl2.pkl:
amends "NonEmptyDef2.pkl"
 
stuff {
    ["key"] {
    }
}

# NonEmptyTest2.pkl:
amends "pkl:test"
import  "pkl:test"
import "NonEmptyTest2.pkl" as confUnderTest
 
examples {
    ["basic-test"] {
        test.catch(() -> confUnderTest.stuff)
    }
}
$ pkl test --overwrite NonEmptyTest2.pkl
module NonEmptyTest2 (file:///tmp/issues/NonEmptyTest2.pkl, line 1)
  basic-test ✍️

$ cat NonEmptyTest2.pkl-expected.pcf 
examples {
  ["basic-test"] {
    "Type constraint `!isEmpty` violated. Value: new Listing {}"
  }
}

The above has been tested using the following versions:

  • Pkl 0.25.3 (Linux 5.15.0-1053-aws, native)
  • Pkl 0.26.0-dev+3a31188 (Linux 5.4.0-177-generic, native)
@translatenix
Copy link
Contributor

I think everything works as expected here. (NonEmptyTest-Inv.pkl isn’t the inverse test.) This should work:

examples {
    ["basic-test"] {
        test.catch(() -> confUnderTest.values) // or: confUnderTest.output.value
    }
}

Here is a more robust test that doesn’t depend on the exact error message:

facts {
    ["basic-test"] {
        test.catchOrNull(() -> confUnderTest.values) != null
    }
}

@holzensp
Copy link
Contributor

holzensp commented Apr 29, 2024

The error seems to suggest that there is no evaluation error here.

This is because there isn't. Where you say confUnderTest, you just refer to the object; nothing's forcing the evaluation of its members. If, instead, you say confUnderTest.output.text or confUnderTest.values.length, you force the evaluation of values and it throws as you'd expect (so test.catch catches it and the test passes).

If we instead embed this list inside a mapping, this works as expected:

Actually; that's not as expected. This is a known bug. Mapping<K,V> is too eager; it should be strict in its keys and lazy in its values, but currently, type checking it forces its values. (See: #406)

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

3 participants