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

Night owl support #11

Open
wants to merge 9 commits into
base: main
Choose a base branch
from
Open

Night owl support #11

wants to merge 9 commits into from

Conversation

sfider
Copy link
Contributor

@sfider sfider commented Nov 22, 2024

Went through the trouble of switching my fork to your repo just to PR this hacky thing. Hope you'll enjoy it :)

@b12f
Copy link
Member

b12f commented Nov 25, 2024

Hey @sfider, thanks so much!

I've ran tests on your branch and it seems to fail:

=================================================================== short test summary info ====================================================================
FAILED tests/test_config.py::test_config_get - AssertionError: assert None == 'foo'
FAILED tests/test_config.py::test_config_getboolean - AssertionError: assert False is True
FAILED tests/test_config.py::test_config_getint - AssertionError: assert None == 42
FAILED tests/test_config.py::test_config_getfloat - AssertionError: assert None == 3.14
FAILED tests/test_config.py::test_config_getlist - AssertionError: assert [] == ['one', 'two ...', 'five six']
FAILED tests/test_watson.py::test_wrong_config - Failed: DID NOT RAISE <class 'watson.watson.ConfigurationError'>
FAILED tests/test_watson.py::test_start_default_tags - AssertionError: assert [] == ['A', 'B']
FAILED tests/test_watson.py::test_start_default_tags_with_supplementary_input_tags - AssertionError: assert ['C', 'D'] == ['C', 'D', 'A', 'B']
========================================================== 8 failed, 338 passed, 77 warnings in 0.74s ==========================================================

Also @teutat3s I thought we enabled checks for this repo, maybe it's not running for PRs with external branches yet? I'll check if I can find out why

@teutat3s
Copy link
Member

If you rebase this on latest main, the CI should trigger as expected.

@sfider
Copy link
Contributor Author

sfider commented Dec 8, 2024

All failing tests assume the config won't be read on Watson object initialization. I broke that assumption by reading 'day_start_hour' from config in Watson.__init__. I think this is the best place to do this, so probably those tests should be redesigned. However, I don't really know pytest nor I have the time to learn it, so I won't be able to complete this PR, sorry.

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.

3 participants