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

User and roles test #3121

Merged
merged 3 commits into from May 18, 2017

Conversation

Projects
None yet
3 participants
@bgeuken
Copy link
Member

bgeuken commented May 18, 2017

No description provided.

[ci] Create :user_nobody factory
In some scenarios you need to assign the nobody user in test cases.

@bgeuken bgeuken force-pushed the bgeuken:user_and_roles_test branch from 95d9731 to beb415b May 18, 2017

@codecov

This comment has been minimized.

Copy link

codecov bot commented May 18, 2017

Codecov Report

Merging #3121 into master will increase coverage by 0.01%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #3121      +/-   ##
==========================================
+ Coverage   88.91%   88.93%   +0.01%     
==========================================
  Files         263      263              
  Lines       17587    17587              
==========================================
+ Hits        15638    15641       +3     
+ Misses       1949     1946       -3
Flag Coverage Δ
#api 84.46% <0%> (+0.78%) ⬆️
#rspec 65.25% <100%> (-0.55%) ⬇️
#webui 64.21% <100%> (ø) ⬆️
if !(real_name.empty? || opt[:short])
printed_name = "#{real_name} (#{user})"
else
if real_name.empty? || opt[:short]

This comment has been minimized.

@Ana06

Ana06 May 18, 2017

Member

This is not a test, this is changing the code. Why? and why is in the test commit? 🤔

This comment has been minimized.

@bgeuken

bgeuken May 18, 2017

Member

I am not the right one to answere this, since I did not write the code. But if you want I can remove that part from the commit.

This comment has been minimized.

@Ana06

Ana06 May 18, 2017

Member

I was discussing something related to that with @mschnitzer and he changed that to check something. So I would say that he commited it by accident.

This comment has been minimized.

@bgeuken

bgeuken May 18, 2017

Member

It's just swapping the condition around (removing the !).

This comment has been minimized.

@bgeuken

bgeuken May 18, 2017

Member

@Ana06 Moved it to a separate commit now

@@ -320,53 +320,42 @@

describe '#user_and_role' do
let(:user) { create(:user) }
let(:nobody) { create(:user_nobody) }
let(:logged_in_user) { create(:user) }
let(:anonymous_user) { create(:user_nobody) }

This comment has been minimized.

@Ana06

Ana06 May 18, 2017

Member

why calling anonymous_user to the nobody_user?

This comment has been minimized.

@bgeuken

bgeuken May 18, 2017

Member

nobody_user is just the factory name. Though internally we have a user with login name '_nobody_', but to me this shouldn't matter for a test.
We have 2 use cases here, one is logged in users and the other one is anonymous or guest users.

@bgeuken bgeuken force-pushed the bgeuken:user_and_roles_test branch from beb415b to 911236a May 18, 2017

@bgeuken bgeuken force-pushed the bgeuken:user_and_roles_test branch from 911236a to ea88557 May 18, 2017

@Ana06
Copy link
Member

Ana06 left a comment

Refactor in the commit message

@Ana06

Ana06 approved these changes May 18, 2017

@bgeuken bgeuken merged commit cb741be into openSUSE:master May 18, 2017

2 checks passed

Hakiri No security warnings were found.
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

@bgeuken bgeuken deleted the bgeuken:user_and_roles_test branch May 18, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment