foreman
foreman copied to clipboard
Fixes #37426 - Rubocop minitest rules fix
upgrading theforeman-rubocop gem to v0.1.2 and that needs some rubocop rules fix.
@ekohl we do want Layout/ArgumentAlignment
cop fix with EnforcedStyle: with_fixed_indentation
here?
I'll raise another PR to split up the work for Lint and Style cops to fix.
@archanaserver historically we've dealt with those bigger changes in separate PRs because it's hard to review multiple cops at the same time. Experience tells us that it's hard to get a consensus and forcing a consensus on all at the same time prolongs the review process a lot. That's why I suggested to split it. the minitest ones are not controversial and can be merged quickly.
I'm not sure why the unit tests fail. Have you taken a look?
I'm not sure why the unit tests fail. Have you taken a look?
Yes, this is the error:
Error:
UsergroupTest#test_0007_should create with valid usergroup:
ActiveRecord::RecordInvalid: Validation failed: Name has already been taken
test/factories/disable_auditing.rb:13:in `block (3 levels) in <top (required)>'
test/factories/disable_auditing.rb:13:in `block (2 levels) in <top (required)>'
test/models/usergroup_test.rb:69:in `block (2 levels) in <class:UsergroupTest>'
test/models/usergroup_test.rb:68:in `each'
test/models/usergroup_test.rb:68:in `block in <class:UsergroupTest>'
coming from here https://github.com/theforeman/foreman/blob/develop/test/factories/disable_auditing.rb#L13 after looking at it the uniqueness validation error pointing to https://github.com/theforeman/foreman/blob/develop/app/models/usergroup.rb#L31 so after looking at the log, i can say that there's an attempt to create a Usergroup
with a name
that has already been taken.
We need to find a way which ensure that each Usergroup
created with this factory has a unique name. I'm still looking how we can encounter the validation error here. @ekohl am i getting it right, do you have any suggestion for the same?
@ekohl any thoughts on this one?
@MariaAga, I've noticed there is a webpack failure here. Would you mind taking a look to see what's causing it?
@archanaserver see #10034 for that.
invalid session id
error in the tests is not related to this pr
@ekohl what does that mean? "@archanaserver archanaserver dismissed ekohl’s stale review via 38a611e"
edit : and what is stale review? i only reverted my last changes.
@archanaserver, there are still two issues related to minitest: https://github.com/theforeman/foreman/actions/runs/8004471095/job/21861933764?pr=9977
@ekohl to pass the redmine check, should I mention same redmine issue number in all commit message or just squash it?
You can use Refs #37426
in the other commits and only keep Fixes
in the last commit.
@ekohl done, thanks :)
@ofedoren @ekohl, can you take a look again? i believe it is ready to be merged!