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

Add pytest tests #6

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

Conversation

schafsam
Copy link
Contributor

@schafsam schafsam commented Feb 10, 2024

Great job your package. I am contributing with pytest tests. What do you think about this?
In order to run these tests, here's what need to happen:

  • Make sure you have pytest installed.
  • Install the mstarpy package by running 'pip install -e .' in your venv.
  • Navigate to the root directory of the package.
  • Run the pytest command to run the tests.

Please note that one of the tests will fail due to the stock returning tickers instead of isin. If I remember correctly, this was also the case for etfs.

@Mael-J Mael-J self-assigned this Feb 11, 2024
@Mael-J
Copy link
Owner

Mael-J commented Feb 11, 2024

Hello,

Thank you for adding tests.

Unfortunately some tests don't pass .

=================================== short test summary info ===================================
FAILED tests/test_fund.py::test_key_stats - AttributeError: 'NoneType' object has no attribute 'find_all'
FAILED tests/test_fund.py::test_one_fund - AttributeError: 'NoneType' object has no attribute 'find_all'
FAILED tests/test_stock.py::test_stock_isin - AssertionError: assert 'TSLA' == 'US88160R1014'
===================== 3 failed, 21 passed, 1 warning in 146.48s (0:02:26) =====================

Also, can you add some descriptions to the tests ?

Thank you.

@schafsam
Copy link
Contributor Author

schafsam commented Feb 13, 2024

Hi

I have been checking the tests and realised that some test fail due to network/response differences.

  • FAILED tests/test_fund.py::test_key_stats - AttributeError: 'NoneType' object has no attribute 'find_all'
  • FAILED tests/test_fund.py::test_one_fund - AttributeError: 'NoneType' object has no attribute 'find_all'

I will check if some delays will mitigate this randomness.

While the follwoing test highlights a difference that probably needs a look. The Stock(...).isin method that returns the ticker and not the isin.

  • FAILED tests/test_stock.py::test_stock_isin - AssertionError: assert 'TSLA' == 'US88160R1014'

This as a heads up for the moment.

@Mael-J
Copy link
Owner

Mael-J commented Feb 19, 2024

Hello,

Yes the isin returned for stocks is not the actual isin.

I saw in your tests, you are testing the name of the funds. This information is quite Volatil.

I am not sure it is pertinent.

Thank you again for adding tests.

I need to take some times to analyse them.

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