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

Add clock interface #7395

Merged
merged 6 commits into from
Jan 9, 2024
Merged

Add clock interface #7395

merged 6 commits into from
Jan 9, 2024

Conversation

merkushin
Copy link
Member

@merkushin merkushin commented Dec 19, 2023

The added interface is similar to PSR-20. The difference is that we can pass the timezone to now() method as it is a common use-case in our codebase.

The implementation is pretty straightforward and returns an immutable instance of current date and time.

There is a default test stub, which always returns the beginning of the Unix epoch (1970-01-01 00:00:00 in UTC). That's a shortcut to simplify testing.

However, if you need to run more complex tests you can replace the clock property in Sensei_Main with a custom mock or just use helper methods. See an example in the updated test.

How it helps with time-based flaky tests? Those tests usually fail unexpectedly when time changed while test was running. To control time in our tests we add an abstraction over the system time function. Since this moment we can set expectations from time without being worry if time has changed.

Now, whenever you want to call time(), you should use Sensei()->clock->now() instead.

Proposed Changes

  • Add clock interface and implementation,
  • Add test stub and test helpers.
  • Update a flaky test and corresponding code to use Clock_Interface.

Testing Instructions

  1. Go to Sensei LMS > Reports.
  2. Check Last Activity column contains correct values.
  3. No errors expected.

New/Updated Hooks

  • sensei_clock_init — Filter clock object.

Pre-Merge Checklist

  • PR title and description contain sufficient detail and accurately describe the changes
  • Acceptance criteria is met
  • Decisions are publicly documented
  • Adheres to coding standards (PHP, JavaScript, CSS, HTML)
  • All strings are translatable (without concatenation, handles plurals)
  • Follows our naming conventions (P6rkRX-4oA-p2)
  • Hooks (p6rkRX-1uS-p2) and functions are documented
  • New UIs are responsive and use a mobile-first approach
  • New UIs match the designs
  • Different user privileges (admin, teacher, subscriber) are tested as appropriate
  • Code is tested on the minimum supported PHP and WordPress versions
  • User interface changes have been tested on the latest versions of Chrome, Firefox and Safari
  • "Needs Documentation" label is added if this change requires updates to documentation
  • Known issues are created as new GitHub issues

@merkushin merkushin added this to the 4.20.1 milestone Dec 19, 2023
@merkushin merkushin requested a review from a team December 19, 2023 06:02
@merkushin merkushin self-assigned this Dec 19, 2023
Copy link

codecov bot commented Dec 19, 2023

Codecov Report

Attention: 1 lines in your changes are missing coverage. Please review.

Comparison is base (49038f8) 50.94% compared to head (1d0f103) 50.96%.
Report is 39 commits behind head on trunk.

Additional details and impacted files

Impacted file tree graph

@@             Coverage Diff              @@
##              trunk    #7395      +/-   ##
============================================
+ Coverage     50.94%   50.96%   +0.01%     
- Complexity    11149    11159      +10     
============================================
  Files           613      614       +1     
  Lines         47062    47085      +23     
  Branches        404      405       +1     
============================================
+ Hits          23975    23996      +21     
- Misses        22760    22762       +2     
  Partials        327      327              
Files Coverage Δ
includes/class-sensei-utils.php 52.78% <100.00%> (+0.07%) ⬆️
includes/clock/class-clock.php 100.00% <100.00%> (ø)
includes/class-sensei.php 24.44% <0.00%> (-0.06%) ⬇️

... and 6 files with indirect coverage changes


Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update d8eaf50...1d0f103. Read the comment docs.

@merkushin merkushin marked this pull request as ready for review December 25, 2023 18:00
renatho
renatho previously approved these changes Dec 27, 2023
Copy link
Contributor

@renatho renatho left a comment

Choose a reason for hiding this comment

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

Thank you @merkushin! Looks good and works well!

Maybe you're already planning it, but it's worth a reminder that it would be a good thing to document in P6rkRX-3bR-p2. :)

includes/class-sensei.php Show resolved Hide resolved
includes/class-sensei.php Outdated Show resolved Hide resolved
@m1r0
Copy link
Member

m1r0 commented Jan 2, 2024

Looks good to me. Thanks!

@merkushin merkushin merged commit b0f41a2 into trunk Jan 9, 2024
24 checks passed
@merkushin merkushin deleted the fix/time-based-tests branch January 9, 2024 05:01
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.

4 participants