Skip to content

[check50_assert] import_checks() does not rewrite assertions #393

@ivanharvard

Description

@ivanharvard

Say you have a folder structure like so:

qux/
   foo/
       .cs50.yaml
       __init__.py
   bar/
       .cs50.yaml
       __init__.py

foo/__init__.py:

import check50

@check50.check()
def test1():
    assert True == False

bar/__init__.py:

foo = __import__("check50").import_checks("../foo")
from foo import *

@check50.check()
def test2():
    assert True == False

If you run check50 with these files, assertion rewriting won't be turned on, so both checks will throw an exception.

Now, you want to enable assertion rewriting. This will cause the tests to still fail, but they fail without throwing an exception and produce a cleaner output. Let's assume you do this via the comment flag # ENABLE_CHECK50_ASSERT = 1. There are three possible choices:

  1. Place the comment flag at the top of bar/__init__.py.
  2. Place the comment flag at the top of foo/__init__.py.
  3. Place the comment flag in both files.

Now imagine you run check50 in bar/.

Using current behavior as a reference, Choice 1 will cause test1 to throw an exception, and test2 to fail as expected. Choice 2 and Choice 3 will cause both test1 and test2 to throw an exception because the assertion rewrite module only focuses on the __init__.py located at the slug, not the imported files.

I believe this is incorrect behavior. Whatever file needs assertion writing should have it declared using the comment flag. For instance, this should run fine and fail as expected without throwing an exception, but it does not:

foo/__init__.py:

# ENABLE_CHECK50_ASSERT = 1
import check50

@check50.check()
def test1():
    assert True == False

bar/__init__.py:

foo = __import__("check50").import_checks("../foo")
from foo import *

And, if bar/__init__.py has its own checks that need rewriting, it should explicitly set this flag (either via the --assertion-rewrite flag or the comment flag). But as of now, the checks in /foo won't be rewritten.
bar/__init__.py:

# ENABLE_CHECK50_ASSERT = 1
foo = __import__("check50").import_checks("../foo")
from foo import *

@check50.check()
def test2():
    assert True == False

Metadata

Metadata

Assignees

No one assigned

    Labels

    4.xIssues relating to check50 4.xbug

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions