Welcome message should use ADMINS set in environment and link to them #97
Conversation
can use this for other use cases too issue: PyBites-Open-Source#84
| @@ -41,10 +43,18 @@ | |||
| What other programming languages do you know and/or use?""" | |||
|
|
|||
|
|
|||
| def _get_admins() -> str: | |||
| admins_env = get_env_var("KARMABOT_ADMINS", default=KARMABOT_ID).split(",") | |||
There was a problem hiding this comment.
I needed to do this here in order to properly mock it in the test
There was a problem hiding this comment.
Do you need to mock it? Can't we just extend
Line 14 in 0eaeefa
There was a problem hiding this comment.
probably, as long as it has multiple admins = more realistic - how to do it please?
There was a problem hiding this comment.
In noxfile.py change it to
"KARMABOT_ADMINS": "FAKE_ADMIN1,FAKE_ADMIN2,FAKE_ADMIN3",
and then use FAKE_ADMIN1 etc. as values in the test
There was a problem hiding this comment.
sweet, thanks, done
There was a problem hiding this comment.
So you always run your tests with poetry run nox -rs tests?
I ask because this is the first test to break poetry run pytest if we do it this way ...
Also, do we need to add poetry run to the nox commands in the README?
There was a problem hiding this comment.
Yes it should always be run with nox. Inside the venv you can just use the nox command.
If you want to run pytest directly you have to have a valid .karmabot file or env variables anyway. That was already a restriction before these changes. As some tests rely on the env variables.
There was a problem hiding this comment.
Gotcha thanks. Anything else I need to change?
| @@ -91,8 +94,29 @@ def test_reply_commands_unknown(capfd, test_message, expected): | |||
| assert out.strip() == expected | |||
|
|
|||
|
|
|||
| def test_welcome_new_user(): | |||
| pass | |||
| @patch("karmabot.commands.welcome.choice") | |||
There was a problem hiding this comment.
tests/test_bot.py
Outdated
| def test_welcome_new_user(choice_mock): | ||
| choice_mock.return_value = "What is your favorite Python module?" | ||
| admins = "U4RTDPKUH,U4TN52NG6,U4SJVFMEG" | ||
| with patch.dict(os.environ, {"KARMABOT_ADMINS": admins}): |
There was a problem hiding this comment.
not leaving env variables to chance in testing
Codecov Report
@@ Coverage Diff @@
## develop #97 +/- ##
===========================================
+ Coverage 54.01% 55.58% +1.57%
===========================================
Files 23 23
Lines 735 743 +8
Branches 87 89 +2
===========================================
+ Hits 397 413 +16
+ Misses 318 312 -6
+ Partials 20 18 -2
Continue to review full report at Codecov.
|
|
Thanks |
This PR includes the changes of https://github.com/PyBites-Open-Source/karmabot/pull/96/files as I needed them here. I tested it in my env and admins are now used from the corresponding env variable and are properly linked: