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

Tests: new Cookie\FormatTest class #740

Merged
merged 1 commit into from
Jun 20, 2022

Conversation

jrfnl
Copy link
Member

@jrfnl jrfnl commented May 10, 2022

... with dedicated tests for the Cookie::__toString(), Cookie::format_for_header() and the Cookie::format_for_set_cookie() methods.

Includes removing some test code from the CookieTest class which is now covered by the tests in this class.

⚠️ Also take note that, IMO, some of these tests highlight potential bugs. I'd like a second opinion on this though before addressing this.

... with dedicated tests for the `Cookie::__toString()`, `Cookie::format_for_header()` and the `Cookie::format_for_set_cookie()` methods.

Includes removing some test code from the `CookieTest` class which is now covered by the tests in this class.

Also take note that, IMO, some of these tests highlight potential bugs. I'd like a second opinion on this though before addressing this.
@jrfnl jrfnl added this to the 2.1.0 milestone May 10, 2022
@jrfnl jrfnl requested a review from schlessera May 10, 2022 22:53
@schlessera
Copy link
Member

Also take note that, IMO, some of these tests highlight potential bugs. I'd like a second opinion on this though before addressing this.

Should we discuss this here or in a separate issue to unblock and merge this PR ?

@jrfnl
Copy link
Member Author

jrfnl commented May 24, 2022

I've opened #743 to discuss the potential bugs.

@jrfnl
Copy link
Member Author

jrfnl commented May 24, 2022

I've rebased the PR after the recent merges and added one additional test case to the dataFormat() data provider - "Has key, empty value".

@schlessera schlessera merged commit 7402c8a into develop Jun 20, 2022
@schlessera schlessera deleted the feature/cookie-format-add-tests branch June 20, 2022 08:10
jrfnl added a commit that referenced this pull request Jun 20, 2022
Hmm... looks like I apparently forgot to push the additional test case I mentioned in #740 (comment) .... oops...

Let's have it anyway.
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.

2 participants