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.

Sign up for GitHub

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

Jump to bottom

Disable non-deterministic flaky test #4295

Merged
merged 1 commit into from
May 31, 2024
Merged

Disable non-deterministic flaky test #4295

merged 1 commit into from
May 31, 2024

Conversation

gaearon
Copy link
Contributor

@gaearon gaearon commented May 31, 2024

Skip test to unbreak main.

I tried fixing it properly but couldn't figure out how/why. This is a bit worrying. However it's still failing if I go back in time on main so this either points to the test already being flawed in some way, or at the implementation already having an issue. It's definitely worth another look but I need to get the CI fixed.

@render Render
Copy link

render bot commented May 31, 2024

Your Render PR Server URL is https://social-app-pr-4295.onrender.com.

Follow its progress at https://dashboard.render.com/web/srv-cpcjrvuct0pc738s5d70.

@github-actions GitHub Actions
Copy link

Old size New size Diff
7.23 MB 7.23 MB 0 B (0.00%)

@gaearon gaearon merged commit 9879159 into main May 31, 2024
6 checks passed
@gaearon gaearon deleted the fix-nd-test branch May 31, 2024 03:09
@janaagaard75
Copy link
Contributor

janaagaard75 commented May 31, 2024 •
edited
Loading

The issue is not the ago function, but the function generating the test data. When the current month has more dates than the previous month, the setMonth() in the snippet becomes invalid, and it is then ignored. So for the current date 2024-05-31, the test date becomes 2024-05-11, and ago() then correctly returns "20d" as the time difference.

new Date(new Date().setMonth(new Date().getMonth() - 1)).setDate(
  new Date().getDate() - 20,
),

Personally, I would create a version of ago() that allows specifying now as a parameter, and then test that version with hard coded dates.

https://www.typescriptlang.org/play/?#code/GYVwdgxgLglg9mABAcwKZQCqoM5QCICGUqAFACZGoBcihxAlDWCALYBGqATogN4BQiQYk7oQnJGFQB3WpRKSZdUhQYA6bOgCyCKAAt502cXKV6qtFG1g9JeogC0iAIz0zG-HIFDBCo8tPm6Eq2DogATAAMADRegvR8AL58fBAIuIgqOIgAvIgA2rGIvsEARJFhACz2EQCs9gDMESX0Md7FcmURldV19U7NrULtxp3dEQBsDU0thcOko1URABwN-TMAusmgkLAIiMBwnCxEwZk0Snb83iJQYkiZqlBwAJIAygDyr1CcMGDItuoQGxcD8-iRos4IvQANyJLaHRAkVJgdKZRBwYAZSjYS6FZHpYi4JQ5IqGYIWLBEuSZVyw7z4uAAG1QqkZcH+ByOJ2ppkQAGpECUHAA+QX8-aHY4eYyE6WoWlwvgAeiViHcVj0iBg2C1yDAh1QZCoKTSTJZbP+cxICx6q2agUwMBYpFcJpRZtZ7IMig65UWvTW6i0On0FVcQA

@gaearon
Copy link
Contributor Author

gaearon commented May 31, 2024

Ah fantastic. Would you like to work on this change?

@janaagaard75
Copy link
Contributor

Ah fantastic. Would you like to work on this change?

Sure. I will try to make a PR within the next week.

@gaearon
Copy link
Contributor Author

gaearon commented May 31, 2024

Thanks!

estrattonbailey added a commit that referenced this pull request May 31, 2024
@estrattonbailey
Merge remote-tracking branch 'origin/main' into samuel/emoji-multiplier
6509943 
* origin/main: (51 commits)
  [🐴] Option to share via chat in post dropdown (#4231)
  [🐴] Record message (#4230)
  [🐴] send record via link in text (Record DMs - base PR) (#4227)
  Use new icons on notifications screen (#4299)
  Composer - fix modals, and other tweaks (#4298)
  Shadows (#4265)
  Change many border widths from `1` to `hairlineWidth` (#4294)
  Add follow button to feed item avatar (#3560)
  Disable non-deterministic flaky test (#4295)
  Don't log downsample warning when unnecessary (#4291)
  [Statsig] Sample noisy events (#4288)
  Bump FontAwesome (#4285)
  Fix scrolling for labeler profiles (#4286)
  Reduce Threadgate button size (#4287)
  put dropdown in fullscreenoverlay on iOS (#4284)
  play haptics before closing modal (#4283)
  match loadmore position to fab (#4280)
  Composer - Use sheet presentation on iOS (#4278)
  don't maintain position whenever there are no parents (#4277)
  Fix native translations on iOS 17.5.1 (#4282)
  ...
estrattonbailey added a commit that referenced this pull request May 31, 2024
@estrattonbailey
Merge remote-tracking branch 'origin/main' into deactivate/disambiguate
e60308e 
* origin/main: (37 commits)
  fix accessibility label in notifications (#4305)
  [🐴] add emoji multiplier prop to RichText and bump it up for DMs (#4229)
  Tweak avi follow button styles (#4304)
  [🐴] Embed backwards compat (#4302)
  [🐴] Add labels to chats (#4293)
  [🐴] Option to share via chat in post dropdown (#4231)
  [🐴] Record message (#4230)
  [🐴] send record via link in text (Record DMs - base PR) (#4227)
  Use new icons on notifications screen (#4299)
  Composer - fix modals, and other tweaks (#4298)
  Shadows (#4265)
  Change many border widths from `1` to `hairlineWidth` (#4294)
  Add follow button to feed item avatar (#3560)
  Disable non-deterministic flaky test (#4295)
  Don't log downsample warning when unnecessary (#4291)
  [Statsig] Sample noisy events (#4288)
  Bump FontAwesome (#4285)
  Fix scrolling for labeler profiles (#4286)
  Reduce Threadgate button size (#4287)
  put dropdown in fullscreenoverlay on iOS (#4284)
  ...
@janaagaard75 janaagaard75 mentioned this pull request Jun 4, 2024
Merged
@janaagaard75
Copy link
Contributor

janaagaard75 commented Jun 11, 2024 •
edited
Loading

The pull request is here: #4364.

Unsure if it's acceptable to change the "months ago" calculation to always use 30 days, but I think this was the best tradeoff. Let me know if you prefer keeping the 30.41675 days.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@haileyok haileyok haileyok approved these changes

@pfrazee pfrazee Awaiting requested review from pfrazee

@estrattonbailey estrattonbailey Awaiting requested review from estrattonbailey

@mozzius mozzius Awaiting requested review from mozzius

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@gaearon @janaagaard75 @haileyok