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

SPIKE: try VCRpy #785

Closed
wants to merge 1 commit into from
Closed

SPIKE: try VCRpy #785

wants to merge 1 commit into from

Conversation

TomasTomecek
Copy link
Member

@packit-as-a-service
Copy link

Congratulations! One of the builds has completed. 🍾

You can install the built RPMs by following these steps:

  • sudo yum install -y dnf-plugins-core on RHEL 8
  • sudo dnf install -y dnf-plugins-core on Fedora
  • dnf copr enable packit/packit-service-packit-785
  • And now you can install the packages.

Please note that the RPMs should be used only in a testing environment.

@softwarefactory-project-zuul
Copy link
Contributor

Build failed.

tests/integration/test_pagure.py Outdated Show resolved Hide resolved
Copy link
Member

@lachmanfrantisek lachmanfrantisek left a comment

Choose a reason for hiding this comment

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

Thanks for the example.

tests/integration/test_pagure.py Show resolved Hide resolved
Comment on lines +50 to +51
# if pagure_token is not set and we're recording, we'll get a token failure,
# but that's expected so no need to take care of it
Copy link
Member

Choose a reason for hiding this comment

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

There is no way how to check the mode? It can cause misleading messages in the CI.

Copy link
Member Author

Choose a reason for hiding this comment

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

the only error messages I got was that token is not valid - pretty clear

when you tamper with the call stack, you will always get misleading error messages

Copy link
Member

Choose a reason for hiding this comment

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

It will be funny with GitHub when you randomly receive API limit errors since it works also unauthenticated.

I don't say it's a big problem, but I already need to describe this problem to various contributors and sometimes also people from the team was confused. (The warnings from API are missing important info -- you need to set a specific env. var. to fix that.)

Comment on lines +54 to +55
filter_headers=["Authorization", "Cookie"],
before_record_response=drop_sensitive_response_data,
Copy link
Member

Choose a reason for hiding this comment

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

The API looks very clear.

Why are you dropping headers twice?

Copy link
Member Author

Choose a reason for hiding this comment

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

request headers, response headers

Copy link
Member

Choose a reason for hiding this comment

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

OK, thanks.

# if pagure_token is not set and we're recording, we'll get a token failure,
# but that's expected so no need to take care of it
with use_cassette(
path=str(Path(__file__).parent / "test_basic_distgit_workflow"),
Copy link
Member

Choose a reason for hiding this comment

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

We still need to use the logic from requre to have one file per test. Or some unittest super class to handle that.

Copy link
Member

Choose a reason for hiding this comment

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

Signed-off-by: Tomas Tomecek <ttomecek@redhat.com>
@softwarefactory-project-zuul
Copy link
Contributor

Build failed.

@TomasTomecek
Copy link
Member Author

I am closing this in the meantime before we come to our decision wrt testing.

@TomasTomecek TomasTomecek deleted the vcrpy branch May 12, 2022 10:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants