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

Support None values #63

Open
12 tasks
rickeylev opened this issue Jul 11, 2023 · 2 comments
Open
12 tasks

Support None values #63

rickeylev opened this issue Jul 11, 2023 · 2 comments
Labels
enhancement New feature or request

Comments

@rickeylev
Copy link
Collaborator

Support for None is very spotty. I think IntSubject supports it, maybe string, but not all of them. None doesn't happen much in rule code per-se, more so in utility functions. It's common enough that not having this makes using rules_testing a bit of a pain.

So implement allowing a None as the actual value and having None passed to equals() and not_equals(). Basic support should be pretty easy (i.e. just asserting None on a None value). Handling a None actual value for all the other assertions might be more involved -- almost all of them assume they are working with a non-None value. We can start with basic and go from there, though.

Subjects to update:

  • Bool
  • Collection
  • DefaultInfo
  • DepsetFile
  • Dict
  • File
  • Int
  • Label
  • Runfiles
  • Str
  • Struct
  • Target
@aignas
Copy link

aignas commented Jul 11, 2023

Another way to do this would be to support a NoneSubject because quite often None is a special value from the business logic point of view and it is definitely not an int or str. Maybe doing env.expect.that_<foo>(actual).is_none() would be also good. If we were not using fluent APIs we could just do env.expect.is_none(actual).

That said supporting None in the equals method is important as well from API ergonomics point of view.

@matts1
Copy link

matts1 commented Feb 21, 2024

See #93 for more details, but why not just solve it with a generic Optional type?

IMO it should be an error to have a none value for something you've declared as a string. If you want that to be possible, it should be declared as a subjects.optional(subjects.str)

def optional(factory):
    def new_factory(value, *, meta):
        # Optimisations could be done here to just pull all the methods from the parent and add the method is_none()
        def some():
            if value == None:
                meta.add_failure("Wanted a value, but got None", None)
            return factory(value, meta = meta)

        def is_none():
            if value != None:
                meta.add_failure("Wanted None, but got a value", value)

        return struct(some = some, is_none = is_none)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

3 participants