Skip to content

Conversation

@weklund
Copy link
Contributor

@weklund weklund commented Jun 29, 2025

  • Use pytest to run all tests
  • Add status badge with static value of 68% after running it locally
  • Add CLAUDE.md to help any developers that use Claude Code
  • Configure codecov to dynamically set status badge

Here was the output from local coverage:

❯ make test-cov
PYTHONPATH=.:test python3 -m pytest test/unit/ test/integration/ --cov=ibind --cov-report=term-missing --cov-report=html
=================================== test session starts ===================================
platform darwin -- Python 3.12.10, pytest-8.4.1, pluggy-1.6.0
rootdir: /Users/weseklund/Projects/forks/ibind
configfile: pytest.ini
plugins: cov-5.0.0
collected 73 items                                                                        

test/unit/support/test_py_utils_u.py ..............                                 [ 19%]
test/integration/base/test_rest_client_i.py ........                                [ 30%]
test/integration/base/test_websocket_client_i.py ........                           [ 41%]
test/integration/client/test_ibkr_client_i.py ................                      [ 63%]
test/integration/client/test_ibkr_utils_i.py ...........                            [ 78%]
test/integration/client/test_ibkr_ws_client_i.py ................                   [100%]

--------- coverage: platform darwin, python 3.12.10-final-0 ----------
Name                                                  Stmts   Miss  Cover   Missing
-----------------------------------------------------------------------------------
ibind/__init__.py                                        14      0   100%
ibind/base/__init__.py                                    0      0   100%
ibind/base/queue_controller.py                           18      0   100%
ibind/base/rest_client.py                               133     26    80%   98, 105, 249-258, 263-265, 277-278, 287-289, 311-314, 317-323, 332
ibind/base/subscription_controller.py                   122     32    74%   64, 66-68, 73-74, 85, 88, 91, 99-100, 153, 172-190, 210, 282, 285, 288-291, 325, 333
ibind/base/ws_client.py                                 216     38    82%   62, 87, 92, 115-117, 121-126, 148, 152-153, 165-166, 187, 193-195, 199-201, 235, 244, 251-253, 313-318, 359, 435-436, 453, 466
ibind/client/__init__.py                                  0      0   100%
ibind/client/ibkr_client.py                              81     38    53%   81-85, 108-111, 124-127, 135-141, 155-157, 188-211, 227-229, 242-243, 246-248, 257-259
ibind/client/ibkr_client_mixins/__init__.py               0      0   100%
ibind/client/ibkr_client_mixins/accounts_mixin.py         4      0   100%
ibind/client/ibkr_client_mixins/contract_mixin.py        25      0   100%
ibind/client/ibkr_client_mixins/marketdata_mixin.py      55     23    58%   54-84, 225, 235, 297
ibind/client/ibkr_client_mixins/order_mixin.py           22     12    45%   110-118, 183-191
ibind/client/ibkr_client_mixins/portfolio_mixin.py        5      0   100%
ibind/client/ibkr_client_mixins/scanner_mixin.py          5      0   100%
ibind/client/ibkr_client_mixins/session_mixin.py         24      1    96%   90
ibind/client/ibkr_client_mixins/watchlist_mixin.py        4      0   100%
ibind/client/ibkr_definitions.py                          6      0   100%
ibind/client/ibkr_utils.py                              217     41    81%   222, 310-313, 316, 425-426, 585-588, 599-602, 621-624, 627-639, 648-654, 666-671
ibind/client/ibkr_ws_client.py                          223     53    76%   272, 275-279, 317, 322-324, 327, 346, 379, 384-385, 397-404, 410-411, 419-430, 435-448, 468, 472, 476, 489, 502, 520, 523, 536
ibind/oauth/__init__.py                                  26     26     0%   1-58
ibind/oauth/oauth1a.py                                  164    164     0%   1-466
ibind/support/__init__.py                                 0      0   100%
ibind/support/errors.py                                   4      0   100%
ibind/support/logs.py                                    63     40    37%   54-73, 98-109, 118-121, 124-125, 128, 131-136, 139-143
ibind/support/py_utils.py                                87     16    82%   307, 323-331, 337-350
ibind/var.py                                             78      2    97%   24-25
-----------------------------------------------------------------------------------
TOTAL                                                  1596    512    68%
Coverage HTML written to dir htmlcov

Copy link
Owner

@Voyz Voyz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great to see this moving forward, thanks for the contribution 🙌 I've left a few small comments, but looks good in general

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey, I understand this is useful to you, but I personally don't use Claude, so I'd think this should be in .gitignore. I'd have hoped for a standard AGENTS.md file to be propagated through the industry, but sadly that doesn't seem to be the case yet 🥲

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As a helper to any contributor, it would be nice to have all the agent type things, but we can leave for another discussion.

I can add all types to gitignore

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can it maybe be added in some sub-directory? Like './AI/CLAUDE.MD` or alike? I'd think it would be awesome to start including these, but I'd rather not see the root dir polluted with settings that are individual to each dev - similar to .iml, .vscode, .venv, etc.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Or agents/CLAUDE.md ?

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ahh that's awesome! yeah, let's go with that!

Comment on lines +1 to +7
[tool:pytest]
testpaths = test
pythonpath = . test
addopts = -v --tb=short
python_files = test_*.py
python_classes = Test*
python_functions = test_* No newline at end of file
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've been trying to use pytest (and Pydantic!) in other projects to transition myself to your camp slowly, and I think these look like they're like this by default. Is there a need to specify any of them?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So there's a few in here that are redundant, but others aren't, I can remove the defaults.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great, thanks! 👍

@weklund
Copy link
Contributor Author

weklund commented Jul 7, 2025

This is exciting! Passing tests on some OS/versions and failing on others! This is exactly the benefit for doing all of this in CI.

Screenshot 2025-07-07 at 16 51 22

Ok time to fix it now haha.

@Voyz
Copy link
Owner

Voyz commented Jul 10, 2025

Awesome, indeed! Kudos for introducing all that CI pipe 🙌

The test_utils import issue is probably related to pytest not finding that file. Any idea on how to fix it? Was it caused by removing any of these config lines from pytest.ini?

@weklund
Copy link
Contributor Author

weklund commented Jul 22, 2025

Looks like we're good! Do you mind one more pass if I've missed anything @Voyz ?

@weklund weklund marked this pull request as ready for review July 22, 2025 01:40
@Voyz
Copy link
Owner

Voyz commented Jul 22, 2025

Pretty much there, just need to remove that badge for now:

 <a href="https://github.com/Voyz/ibind">
        <img src="https://img.shields.io/badge/coverage-68%25-green.svg"/>
    </a>

@weklund
Copy link
Contributor Author

weklund commented Jul 23, 2025

@Voyz Ah yes! Here we go.

@Voyz
Copy link
Owner

Voyz commented Jul 23, 2025

Sweet! LGTM, thanks 🙌

@Voyz Voyz merged commit 55c4215 into Voyz:master Jul 23, 2025
8 checks passed
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

Successfully merging this pull request may close these issues.

2 participants