CODEWITHSUNDEEP
pythonpytestpython-unittest
ambertaka
ambertaka

Reputation: 33

len function not behaving as expected

I'm building a function that's applying an additional tag to an aws instance based on a dict of tags that are passed in.

Expected behavior: When more than one TEST_CUSTOMER_ID is are passed in, the function should return the following dictionary of tags:

{'foo': 'bar', 'Account': 'shared'}

The current behavior of the unit test function is only returning:

{'foo': 'bar', 'Account': [['test_customer_id1', 'test_customer_id2']]}

How can I fix this?

def get_acct_value(tags, customer_id, ceid):
    customer_id = [customer_id]
    
    if "Account" in tags:
        if tags["Account"] == customer_id:
            pass
        else:
            if len(customer_id) > 1:
                tags["Account"] = "shared"
            elif len(customer_id) == 1:
                tags["Account"] = customer_id
            else:
                raise exceptions.CustomerNotFoundError(f"No customer(s) found on {ceid}")
    else:
        if len(customer_id) > 1:
            tags["Account"] = "shared"
        elif len(customer_id) == 1:
            tags["Account"] = customer_id
        else:
            raise exceptions.CustomerNotFoundError(f"No customer(s) found on {ceid}")
    return tags

Unit test:

TAGS_NO_VALUE = {'foo': 'bar'}
TEST_CUSTOMER_ID_LIST = ["test_customer_id1", "test_customer_id2"]
TEST_CEID = "test_ceid"


def test_get_account_value_customer_list():
    response = tagging.get_acct_value(TAGS_NO_VALUE, TEST_CUSTOMER_ID_LIST, TEST_CEID)

    print(response)

Other unit tests:

All three tests should return: {'Account': customer_id, 'foo': 'bar'}

TEST_CUSTOMER_ID = "test_customer_id"
TAGS_UNEXPECTED_VALUE = {'Account': '', 'foo': 'bar'} 
TAGS_EXPECTED_VALUE = {'Account': customer_id, 'foo': 'bar'} 

def test_get_acct_value_no_value():

    response = tagging.get_acct_value(TAGS_NO_VALUE, 
TEST_CUSTOMER_ID, TEST_CEID)

    print(response)


def test_get_acct_value_unexpected_value():

    response = tagging.get_acct_value(TAGS_UNEXPECTED_VALUE, TEST_CUSTOMER_ID, TEST_CEID)

    print(response)

def test_get_acct_value_expected_value():

    response = tagging.get_acct_value(TAGS_EXPECTED_VALUE, TEST_CUSTOMER_ID, TEST_CEID)

    print(response)

Upvotes: 3

Views: 114

Answers (1)

Nathaniel Ford
Nathaniel Ford

Reputation: 21249

I think you're complicating yourself a great deal here. Let's break out this function differently:

def get_acct_value(tags, customer_ids, ceid):
    if len(customer_ids) == 0:
        raise exceptions.CustomerNotFoundError(f"No customer(s) found on {ceid}")
    tag = "shared" if len(customer_ids) > 1 else customer_ids[0]
    tags["Account"] = tag
    return tags

First, we know that if customer_ids, a list, is empty, we should raise an exception. Do this first. It's labelled 'bounds checking', and should be done before you try and process any data - that way you don't have to redo it on every branch of your code.

Secondly, we know that if the list is greater than one, we want our tag to be 'shared', meaning we have more than one customer id. Let's set a temporary variable with the name tag with 'shared' if we have a list greater than one. If the list is exactly one, we set it to the only available customer id.

Finally, we do the actual work - setting the account to the tag we have selected. Lines 4 and five could be combined to tags["Account"] = "shared" if len(customer_ids) > 1 else customer_id[0].

Notably, your proximal issue is that the type of customer_ids being passed in must be a list. If it is a solitary value then you'll have an issue. You try to solve this by just casting it to a list, but if you want to accept either a list or a single value, you're better doing something like this:

customer_ids = customer_ids if isinstance(customer_ids, list) else [customer_ids]

This would result in something like:

def get_acct_value(tags, customer_ids, ceid):
    customer_ids = list() if customer_ids is None else customer_ids
    customer_ids = customer_ids if isinstance(customer_ids, list) else [customer_ids]
    print(f"type={type(customer_ids)} {customer_ids=}")
    if len(customer_ids) == 0:
        raise exceptions.CustomerNotFoundError(f"No customer(s) found on {ceid}")
    tags["Account"] = "shared" if len(customer_ids) > 1 else customer_ids[0]
    return tags

I've added an initial check for customer_ids to ensure it is not None, which would break your second check (to convert the value to a list if it is not one), since list(None) throws a TypeError.

Note that I would sooner name this function update_account_tags() or something like that, since it returns no value, just a dictionary of tags, which has an updated value for account.

Some guidance: if you find yourself doing a check, if a in b, where b is a dictionary, and you're planning to do something with a, the best thing to do is use the dictionary's function get().

v = b.get(a, my_default)

my_default here can be whatever you want, and by default is None. So these are equivalent:

v = b.get(a)
v = b[a] if a in b else None

Secondly, if you find yourself in a situation where you're doing a check like this:

if tags["Account"] == customer_id:
    pass
else:
   tags["Account"] = customer_id

You might as well simply do this:

tags["Account"] = customer_id

The result is the same, and it's equally computationally complex. (If customer_id is replaced with a function like get_customer_id() this may not be entirely true, but as a first instinct it'll do you well.)

Upvotes: 3

Related Questions