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

fixtures: add second user #113

Merged
merged 4 commits into from
Sep 4, 2024

Conversation

DaanRosendal
Copy link
Member

Copy link

codecov bot commented Nov 4, 2023

Codecov Report

Attention: Patch coverage is 0% with 73 lines in your changes missing coverage. Please review.

Project coverage is 3.64%. Comparing base (023adcf) to head (40d3c6e).
Report is 4 commits behind head on master.

Files with missing lines Patch % Lines
pytest_reana/fixtures.py 0.00% 72 Missing ⚠️
pytest_reana/version.py 0.00% 1 Missing ⚠️
Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff            @@
##           master    #113      +/-   ##
=========================================
- Coverage    4.80%   3.64%   -1.16%     
=========================================
  Files           5       5              
  Lines         208     274      +66     
=========================================
  Hits           10      10              
- Misses        198     264      +66     
Files with missing lines Coverage Δ
pytest_reana/plugin.py 0.00% <ø> (ø)
pytest_reana/version.py 0.00% <0.00%> (ø)
pytest_reana/fixtures.py 2.75% <0.00%> (-0.97%) ⬇️

Scope: function

This fixture creates a user with UUID
``11111111-1111-1111-1111-111111111111``, ``email`` `info2@reana.io`
Copy link
Member

Choose a reason for hiding this comment

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

Cosmetics: it does not look logical to have a "second user" with "1111" ID.

Perhaps we could profit from the occasion of introducing new test users to clean the setup a bit.

For example, the current "default user" is actually an "admin user", created by default. We don't really have many testing scenarios that would be admin-only, but we could start distinguishing them, now that we shall have regular users. I.e. those tests that need to test for admin functionality would use the "admin user", and other tests (including workflow sharing) would use "regular users" by default.

This would give something lik:

User Role ID Email Token
user0 admin user 00000 user0@reana.io user0token
user1 first regular user 11111 user1@reana.io user1token
user2 second regular user 22222 user2@reana.io user2token

It may be somewhat painful to rename "default_user" to "user0" in many tests, but I guess it might be worth it.

Copy link
Member

Choose a reason for hiding this comment

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

BTW please rebase against latest master, and you could also prepare a release commit, because we would need to release a new version and in depending PRs we would need to ask for higher version of pytest-package, otherwise the PRs would not pass (for your workflow sharing features), but also other components would start to fail (because there would be no default_user anymore).

Copy link
Member Author

Choose a reason for hiding this comment

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

Did the rebase. I am not sure how to do a release commit though

@DaanRosendal DaanRosendal force-pushed the fixture/second-user branch 3 times, most recently from bbaa87d to 6a1c92e Compare November 9, 2023 19:37
Copy link
Member

@mdonadoni mdonadoni left a comment

Choose a reason for hiding this comment

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

Looks good! If we need more users in the future, we can consider having just a single fixture that can be called multiple times to create as many users as needed.

This PR needs to be rebased and the commit messages need to be modified to follow the new commit convention. This PR will also need a release commit.

@DaanRosendal DaanRosendal force-pushed the fixture/second-user branch 4 times, most recently from fcc2fb2 to 5445175 Compare March 11, 2024 20:08
DaanRosendal added a commit to DaanRosendal/pytest-reana that referenced this pull request Mar 11, 2024
BREAKING CHANGE: This commit replaces the default_user fixture with
user1 and user2 fixtures. The default_user fixture is now deprecated and
will be removed in the next release. (reanahub#113)

Related to reanahub/reana-client#680
DaanRosendal added a commit to DaanRosendal/pytest-reana that referenced this pull request Mar 14, 2024
BREAKING CHANGE: This commit replaces the default_user fixture with
user1 and user2 fixtures. The default_user fixture is now deprecated and
will be removed in the next release.

Related to reanahub/reana-client#680
DaanRosendal added a commit to DaanRosendal/pytest-reana that referenced this pull request Mar 14, 2024
DaanRosendal added a commit to DaanRosendal/pytest-reana that referenced this pull request Mar 18, 2024
BREAKING CHANGE: This commit replaces the default_user fixture with
user1 and user2 fixtures. The default_user fixture is now deprecated and
will be removed in the next release.

Related to reanahub/reana-client#680
DaanRosendal added a commit to DaanRosendal/pytest-reana that referenced this pull request Mar 18, 2024
mdonadoni pushed a commit to DaanRosendal/pytest-reana that referenced this pull request Aug 20, 2024
BREAKING CHANGE: This commit replaces the default_user fixture with
user1 and user2 fixtures. The default_user fixture is now deprecated and
will be removed in the next release.

Related to reanahub/reana-client#680
mdonadoni pushed a commit to DaanRosendal/pytest-reana that referenced this pull request Aug 20, 2024
@mdonadoni
Copy link
Member

Latest version before rebase is 8afface

This needs to be released to make the tests of other components pass.

mdonadoni added a commit to DaanRosendal/pytest-reana that referenced this pull request Aug 20, 2024
DaanRosendal and others added 3 commits August 28, 2024 09:13
BREAKING CHANGE: This commit replaces the default_user fixture with
user1 and user2 fixtures. The default_user fixture is now deprecated and
will be removed in the next release.

Related to reanahub/reana-client#680
@mdonadoni
Copy link
Member

Ready to be reviewed, squashed, merged and released!

@tiborsimko tiborsimko merged commit 40d3c6e into reanahub:master Sep 4, 2024
13 checks passed
Copy link
Member

@tiborsimko tiborsimko left a comment

Choose a reason for hiding this comment

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

Works nicely 👍

- r-client #692
- r-client-go #153
- r-ui #375
- r-server #658
- r-workflow-controller #552
- r-commons #429
- r-db #216

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