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

(MODULES-3539) Allow @ in username #1155

Merged
merged 5 commits into from
Jan 14, 2019
Merged

Conversation

Fogelholk
Copy link
Contributor

Rebase of #1097

Allows having @ in the username (for example 'foo@bar'@'localhost')

If I did it wrong (Sorry, I don't know Github too well), I can scrap my PRs and start over :)

Allows having @ in the username (for example 'foo@bar'@'localhost')
@david22swan
Copy link
Member

@Fogelholk No error shown in the adhoc run, but to get this merged it would still need docs changes and test coverage to ensure that people are aware of it and it always works as intended.

@Fogelholk
Copy link
Contributor Author

I'm not sure how to add test coverage, should I create a new block in puppetlabs-mysql/blob/master/spec/acceptance/types/mysql_user_spec.rb with, for example, the username 'foo@bar@localhost'?

@david22swan
Copy link
Member

@Fogelholk That sounds good to me

@Fogelholk
Copy link
Contributor Author

@david22swan Thanks for all the help so far, I hope I've done everything correctly :)

@david22swan
Copy link
Member

@Fogelholk Unfortunately the test's you added have failed against travis. I've posted the error's below:

Failures:
  1) mysql_user using foo@bar@localhost adding user works without errors
     Failure/Error: execute_manifest(pp_three, catch_failures: true)
     Beaker::TestmodeSwitcher::UnacceptableExitCodeError:
       Unacceptable exit code returned: 4. Acceptable code(s): 0, 2
       
     # ./vendor/bundle/ruby/2.5.0/gems/beaker-testmode_switcher-0.4.1/lib/beaker/testmode_switcher/runner_base.rb:31:in `handle_puppet_run_returned_exit_code'
     # ./vendor/bundle/ruby/2.5.0/gems/beaker-testmode_switcher-0.4.1/lib/beaker/testmode_switcher/beaker_runners.rb:138:in `execute_manifest_on'
     # ./vendor/bundle/ruby/2.5.0/gems/beaker-testmode_switcher-0.4.1/lib/beaker/testmode_switcher/beaker_runners.rb:114:in `execute_manifest'
     # ./vendor/bundle/ruby/2.5.0/gems/beaker-testmode_switcher-0.4.1/lib/beaker/testmode_switcher/dsl.rb:13:in `block (2 levels) in <module:DSL>'
     # ./spec/acceptance/types/mysql_user_spec.rb:115:in `block (4 levels) in <top (required)>'
  2) mysql_user using foo@bar@localhost adding user finds the user #stdout
     Failure/Error: expect(r.stdout).to match(%r{^1$})
       expected "" to match /^1$/
       Diff:
       @@ -1,2 +1,2 @@
       -/^1$/
       +""
       
       
     # ./spec/acceptance/types/mysql_user_spec.rb:120:in `block (5 levels) in <top (required)>'
     # ./vendor/bundle/ruby/2.5.0/gems/beaker-4.4.0/lib/beaker/dsl/helpers/host_helpers.rb:93:in `block in on'
     # ./vendor/bundle/ruby/2.5.0/gems/beaker-4.4.0/lib/beaker/shared/host_manager.rb:130:in `run_block_on'
     # ./vendor/bundle/ruby/2.5.0/gems/beaker-4.4.0/lib/beaker/dsl/patterns.rb:37:in `block_on'
     # ./vendor/bundle/ruby/2.5.0/gems/beaker-4.4.0/lib/beaker/dsl/helpers/host_helpers.rb:63:in `on'
     # ./vendor/bundle/ruby/2.5.0/gems/beaker-4.4.0/lib/beaker/dsl/helpers/host_helpers.rb:125:in `shell'
     # ./spec/acceptance/types/mysql_user_spec.rb:119:in `block (4 levels) in <top (required)>'
Finished in 4 minutes 38.3 seconds (files took 1 minute 49.19 seconds to load)
137 examples, 2 failures

@Fogelholk
Copy link
Contributor Author

@david22swan There we go, I haven't added users with @ in their username before, I had those users created before I added puppet, but it seems to work when I try locally and automatic tests seems happy :)

@david22swan
Copy link
Member

@Fogelholk Looks good to me. :)
Thanks for putting the effort in.

@david22swan david22swan merged commit 54af565 into puppetlabs:master Jan 14, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants