fabric8-services / fabric8-auth

Identity and Access Management for fabric8 services
https://auth.openshift.io/api/status
Apache License 2.0
14 stars 26 forks source link

Temporary config change / enable workers for testing #818

Closed sbryzak closed 5 years ago

sbryzak commented 5 years ago

https://jira.coreos.com/browse/ODC-146

This PR enables step 2 of the testing plan described in the above issue.

Still to-do: limit the user deactivation list to a pre-selected list of e-mail addresses.

alien-ike commented 5 years ago

Ike Plugins (test-keeper)

Thank you @sbryzak for this contribution!

It seems that this PR already contains some added or changed tests. Good job!

Your plugin configuration is stored in the file.

codecov[bot] commented 5 years ago

Codecov Report

Merging #818 into master will decrease coverage by 0.08%. The diff coverage is 64.28%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #818      +/-   ##
==========================================
- Coverage   78.51%   78.42%   -0.09%     
==========================================
  Files          98       98              
  Lines        9158     9168      +10     
==========================================
  Hits         7190     7190              
- Misses       1438     1445       +7     
- Partials      530      533       +3
Impacted Files Coverage Δ
authentication/account/service/user.go 76.05% <60%> (-0.8%) :arrow_down:
configuration/configuration.go 83.56% <75%> (+0.11%) :arrow_up:
...ication/account/worker/user_deactivation_worker.go 61.29% <0%> (-22.59%) :arrow_down:
...rovider/service/authentication_provider_service.go 73.33% <0%> (+1.11%) :arrow_up:

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 14148e5...a122771. Read the comment docs.

sbryzak commented 5 years ago

@nurali-techie could you provide a list of e-mail addresses you'd like to limit the deactivation testing to?

nurali-techie commented 5 years ago

@sbryzak one more thing, I think that this test will consume lot of time and if something goes wrong it will be difficult to debug in prod-preview. I suggest to add few logger statement so in case things don't work you know well in advance rather waiting for worker timer.

sbryzak commented 5 years ago

@nurali-techie You were right about the GetXXX() methods, they used getInt() to retrieve the configuration parameter values, meaning the fractional values wouldn't work. I changed them to getFloat64() for the time being, which should now all the config values to work.

nurali-techie commented 5 years ago

@sbryzak yes, but apart from getInt() to getFloat64(), please observe the value is getting multiplied with 24 * time.Hour. Approving the PR assuming you have run the code and check the values :+1:

sbryzak commented 5 years ago

@nurali-techie yes, there turns out to be some constraints when using time.Duration() so I've just overridden the configuration functions altogether instead of trying to set the defaults. Looks like this works now (tested locally).