-
Notifications
You must be signed in to change notification settings - Fork 50
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
adding real auth to integration tests #352
Conversation
we should remove |
yes once the other changes I'm adding work, but still waiting to make sure integration tests pass locally |
are we skipping auth for circleci integration tests? |
…gine into ianmacleod/use_real_auth
integration_tests/test_fine_tunes.py
Outdated
@@ -1,7 +1,10 @@ | |||
""" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this test was already not functioning so I commented everything out - todo later is come through and fix it so that it works.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Mind creating a follow-up ticket for this?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yeah sounds good!
integration_tests/test_fine_tunes.py
Outdated
@@ -1,7 +1,10 @@ | |||
""" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Mind creating a follow-up ticket for this?
integration_tests/rest_api_utils.py
Outdated
USER_ID_1, | ||
) | ||
# Use the scale-launch-integration-tests id | ||
USER_ID_0 = os.getenv("TEST_USER_ID") # type: ignore |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
set some default value
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
and i think None
USER_ID_0 caused integration test to fail
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
agreed, I set a default value of "fakeuser", this should pass CI since we don't use the auth repo in CI currently
integration_tests/test_fine_tunes.py
Outdated
|
||
# cancel_response = cancel_fine_tune_by_id(fine_tune_id, USER_ID_0) | ||
# assert cancel_response["success"] | ||
# list_response_1 = list_fine_tunes(USER_ID_0) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
there should be some @pytest.skip(reason="test doesn't work")
thing that you can do to skip the test (as opposed to commenting it out)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yeah let me add this too - although I will say the test was already doing nothing, just asserting that the list of fine tunes for the user was empty
…gine into ianmacleod/use_real_auth
No description provided.