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

Fixes #34182 - fix error message when import attribute is missing #9862

Merged
merged 1 commit into from
Dec 20, 2021

Conversation

evgeni
Copy link
Member

@evgeni evgeni commented Dec 16, 2021

What are the changes introduced in this pull request?

Corrects error message when there are import attributes missing.

Otherwise you get the following error:

undefined local variable or method `import_attributes' for #<Katello::Api::V2::RepositoriesController:0x000055ecb351d600>
Did you mean?  import_attribute

Or, fixing that:

wrong number of arguments (given 2, expected 1)

Instead of e.g. "ostree_repository_name is required"

Considerations taken when implementing this change?

None, I just want the error to be correct ;-)

What are the testing steps for this pull request?

Trigger import_uploads for an OSTree content w/o setting ostree_repository_name

@theforeman-bot
Copy link

theforeman-bot commented Dec 16, 2021

Issues: #34182

@evgeni evgeni changed the title Refs #33292 - correct error message when import attribute is missing Refs #33292 - fix error message when import attribute is missing Dec 16, 2021
@evgeni
Copy link
Member Author

evgeni commented Dec 16, 2021

I do wonder why RepositoryControllerTest::test_import_uploads_ostree_fails_without_required_params didn't catch that?!

Oooh, it's skipped. Should it not be skipped anymore? We'll find out in #9863

@evgeni
Copy link
Member Author

evgeni commented Dec 17, 2021

Error Message

Expected response to be a <422: Unprocessable Entity>, but was a <500: Internal Server Error>...

Stacktrace

Failure:
test_import_uploads_ostree_fails_without_required_params(Minitest::Result) [/home/jenkins/workspace/katello-pr-test/test/controllers/api/v2/repositories_controller_test.rb:993]:
Expected response to be a <422: Unprocessable Entity>, but was a <500: Internal Server Error>
Response body: {"displayMessage":"undefined local variable or method `import_attributes' for #\u003cKatello::Api::V2::RepositoriesController:0x0000000017ec0da8\u003e\nDid you mean?  import_attribute","errors":["undefined local variable or method `import_attributes' for #\u003cKatello::Api::V2::RepositoriesController:0x0000000017ec0da8\u003e\nDid you mean?  import_attribute"]}.
Expected: 422
  Actual: 500

Now we're talking!

Otherwise you get the following error:

    undefined local variable or method `import_attributes' for #<Katello::Api::V2::RepositoriesController:0x000055ecb351d600>
    Did you mean?  import_attribute

Or, fixing that:

    wrong number of arguments (given 2, expected 1)

Instead of e.g. "ostree_repository_name is required"
@evgeni
Copy link
Member Author

evgeni commented Dec 17, 2021

Right, so I can't enable the controller test w/o offending all the recorded VCR tapes. And I am more in 🎄 than in 📼 mood ;)

@evgeni
Copy link
Member Author

evgeni commented Dec 17, 2021

error is during assets precompile, which is unrelated

Copy link
Member

@jlsherrill jlsherrill left a comment

Choose a reason for hiding this comment

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

ACK for the code change, should we open a new issue so we can more easily backport the issue?

@jlsherrill jlsherrill changed the title Refs #33292 - fix error message when import attribute is missing Fixes #34182 - fix error message when import attribute is missing Dec 20, 2021
@jlsherrill
Copy link
Member

[test katello]

@jlsherrill jlsherrill merged commit 9e6333a into Katello:master Dec 20, 2021
@evgeni evgeni deleted the ref33292 branch December 20, 2021 17:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants