Skip to content

Conversation

@iulianmac
Copy link
Contributor

@iulianmac iulianmac commented Apr 13, 2022

What:

Debug why inspect.ismethod() did not work in cli.py and update steps in contribution guide on building the docker image for development.

Why:

Resolves a FIXME comment and updates development guide on building Docker image for dev.

How:

Executed all unit-tests.

Done the following small test:

import inspect

class C(object):
    def foo(self):
        pass
>>> callable(getattr(C, "foo")) # What is now in code
True
>>> inspect.ismethod(getattr(C, "foo")) # Probably what caused the "FIXME" comment
False
>>> inspect.ismethod(getattr(C(), "foo")) # Propsed fix
True

Probably because inspect.ismethod() Return True if the object is a bound method written in Python.

>>> C.foo
<function C.foo at 0x104aa6ca0>
>>> C().foo
<bound method C.foo of <__main__.C object at 0x104ab87f0>>

Risks:

Potential cases not covered by unit-tests today.

Checklist:

  • Added tests, if you've added code that should be tested (N/A)
  • Updated the documentation, if you've changed APIs (N/A)
  • Ensured the test suite passes
  • Made sure your code lints
  • Completed the Contributor License Agreement ("CLA")

@facebook-github-bot facebook-github-bot added the CLA Signed Do not delete this pull request or issue due to inactivity. label Apr 13, 2022
@iulianmac iulianmac marked this pull request as draft April 13, 2022 18:53
@iulianmac iulianmac marked this pull request as ready for review April 13, 2022 19:06
@iulianmac
Copy link
Contributor Author

iulianmac commented Apr 13, 2022

Seems that CI is failing for Python3.6 but looks unrelated to my changes. I was able to reproduce the failing case from "main" branch as well with python3.6.

>       assert "passed, 4 errors" in result.stdout.str()
E       AssertionError: assert 'passed, 4 errors' in ''
E        +  where '' = <bound method LineMatcher.str of <_pytest.pytester.LineMatcher object at 0xffffac8979e8>>()
E        +    where <bound method LineMatcher.str of <_pytest.pytester.LineMatcher object at 0xffffac8979e8>> = <_pytest.pytester.LineMatcher object at 0xffffac8979e8>.str
E        +      where <_pytest.pytester.LineMatcher object at 0xffffac8979e8> = <RunResult ret=ExitCode.USAGE_ERROR len(stdout.lines)=0 len(stderr.lines)=5 duration=0.02s>.stdout

/root/src/TestSlide/pytest-testslide/tests/test_pytest_testslide.py:129: AssertionError

@iulianmac
Copy link
Contributor Author

I've put out #332 as an attempt to fix CI tests.

@facebook-github-bot
Copy link

@deathowl has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CLA Signed Do not delete this pull request or issue due to inactivity.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants