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.

Sign up for GitHub

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

Jump to bottom

test: use kili_admin for tests that require org admin priviledges #1524

Draft
wants to merge 6 commits into
base: main
Choose a base branch
from
Draft

test: use kili_admin for tests that require org admin priviledges #1524

wants to merge 6 commits into from

Conversation

PierreLeveau
Copy link
Collaborator

@PierreLeveau PierreLeveau commented Oct 5, 2023 •
edited
Loading

@PierreLeveau PierreLeveau changed the title tests: moved e2e according to user type test: moved e2e according to user type Oct 5, 2023
@PierreLeveau PierreLeveau force-pushed the feature/ml-1419-lower-permission-e2e-tests branch from 9392828 to de6751a Compare October 5, 2023 15:03
@PierreLeveau PierreLeveau requested a review from a team October 5, 2023 15:15
@PierreLeveau PierreLeveau marked this pull request as ready for review October 5, 2023 15:16
@PierreLeveau PierreLeveau self-assigned this Oct 5, 2023
Jonas1312
Jonas1312 requested changes Oct 5, 2023
View reviewed changes
Copy link
Contributor

@Jonas1312 Jonas1312 left a comment •
edited
Loading

Choose a reason for hiding this comment

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

instead of moving all tests, we can create two kili client fixtures kili and kili_admin in tests/e2e/conftest.py

the kili_admin client use the kili_api_key_admin, and the normal kili client uses the kili_api_key

by default, all e2e tests use the kili fixture (it's already the case)

and if some e2e tests require admin rights, we pass the kili_admin client fixture

tests/e2e/test_tags.py Show resolved Hide resolved
@PierreLeveau PierreLeveau force-pushed the feature/ml-1419-lower-permission-e2e-tests branch from b44204c to f18b5ef Compare October 6, 2023 07:28
@PierreLeveau PierreLeveau changed the title test: moved e2e according to user type test: use kili_admin for tests that require org admin priviledges Oct 6, 2023
@Jonas1312
Copy link
Contributor

do the e2e tests with exports fail when using the kili user client? (they should)

@PierreLeveau
Copy link
Collaborator Author

do the e2e tests with exports fail when using the kili user client? (they should)

there is no export e2e test

@Jonas1312
Copy link
Contributor

Jonas1312 commented Oct 6, 2023 •
edited
Loading

do the e2e tests with exports fail when using the kili user client? (they should)

there is no export e2e test

notebooks

the current solution only applies to python-modules e2e tests

I wonder if we should do the same for notebooks too

In the ci at the notebook steps, we should make the KILI_API_KEY with lower rights too

@Jonas1312 Jonas1312 marked this pull request as draft October 9, 2023 07:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@theodu theodu theodu approved these changes

@Jonas1312 Jonas1312 Jonas1312 approved these changes

At least 2 approving reviews are required to merge this pull request.

Assignees

@PierreLeveau PierreLeveau

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@PierreLeveau @Jonas1312 @theodu