Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

[HttpKernel] Remove time-sensitivity fromInlineFragmentRendererTest#42824

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Conversation

@herndlm
Copy link
Contributor

QA
Branch?4.4
Bug fix?no
New feature?no
Deprecations?no
TicketsN/A
LicenseMIT
Doc PRN/A

I noticed this test failing in#42823 and this is my attempt to remove the time-sensitivity there.

@herndlm
Copy link
ContributorAuthor

Is there a better way to compare ignoring parts? This is the best I could come up with..

@herndlm
Copy link
ContributorAuthor

herndlm commentedSep 1, 2021•
edited
Loading

Strange, I don't understand why the 7.4 high-deps test would still fail comparing those responses. I tested locally with a sleep to be sure this works 🤔

Update: this must be because of how the high-deps work which I did not fully understand yet.. Would be great if exactly that error is not happening any more after this PR got merged.

Update2: I think I got it now. The high-deps mean in this context that checks are done with my branch as dependency using Symfony 3.4. So even if this PR gets merged, the high-deps CI job for 4.4 PRs might still make problems hmm.
I guess trying to remove time-sensivity by not comparing timestamps, especially if they are floats, does make sense. But I was hoping to fix the 4.4 high-deps failures :/

@herndlmherndlmforce-pushed thebugfix/remove-time-sensitivity-from-inline-fragment-renderer-test branch 2 times, most recently from0428e66 to70e1bc0CompareSeptember 3, 2021 11:20
Copy link
Member

@nicolas-grekasnicolas-grekas left a comment

Choose a reason for hiding this comment

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

(the failure are false positives when testing 3.4 with patched deps)

@nicolas-grekas
Copy link
Member

Any idea why these tests fail? Does that follow a recent PR?

@herndlm
Copy link
ContributorAuthor

Any idea why these tests fail? Does that follow a recent PR?

no clue unfortunately, I did not further look into it. I just saw that failure in 4.4 PRs and thought "oh, something time-sensitive, let's fix it" :/

@herndlmherndlmforce-pushed thebugfix/remove-time-sensitivity-from-inline-fragment-renderer-test branch from70e1bc0 tob5831b6CompareSeptember 22, 2021 07:52
xabbuh added a commit that referenced this pull requestOct 1, 2021
This PR was merged into the 4.4 branch.Discussion----------[HttpKernel] Relax some transient tests| Q             | A| ------------- | ---| Branch?       | 4.4| Bug fix?      | no| New feature?  | no| Deprecations? | no| Tickets       | -| License       | MIT| Doc PR        | -As observed in#42824 and more recently#43138, these tests randomly break for some reason.Replaces#42824Commits-------30fa29f [HttpKernel] Relax some transient tests
@chalasr
Copy link
Member

Closing in favor of#43264. Thank you for the PR.

herndlm reacted with thumbs up emoji

@chalasrchalasr closed thisOct 5, 2021
@herndlmherndlm deleted the bugfix/remove-time-sensitivity-from-inline-fragment-renderer-test branchOctober 5, 2021 14:03
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment

Reviewers

@nicolas-grekasnicolas-grekasnicolas-grekas approved these changes

Assignees

No one assigned

Projects

None yet

Milestone

4.4

Development

Successfully merging this pull request may close these issues.

4 participants

@herndlm@nicolas-grekas@chalasr@carsonbot
[8]ページ先頭
©2009-2025 Movatter.jp