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

Fix for flakey tests #134

Merged
merged 2 commits into from
Apr 23, 2024
Merged

Fix for flakey tests #134

merged 2 commits into from
Apr 23, 2024

Conversation

ericaporter
Copy link
Contributor

@ericaporter ericaporter commented Apr 22, 2024

We are getting flakey tests in this file with the checksum_calculated_at being 1s after the expectation. This PR

  • Updates @checksum_calculated_at to store a Time object instead of a
    string to ensure compatibility with Timecop.freeze
  • Modifies the before block to fetch current_timestamp from the database,
    convert it to the specified timezone, and directly use this Time object
    with Timecop.freeze
  • Ensure Timecop is used effectively by freezing time at the exact
    moment defined by the database's current timestamp in the specified
    timezone

@ericaporter ericaporter changed the title Fix for flakey tests - make time static Fix for flakey tests Apr 23, 2024
Copy link
Collaborator

@asatwal asatwal left a comment

Choose a reason for hiding this comment

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

LGTM - Minor comment regarding use of instance variables in tests.

@@ -68,7 +67,11 @@
let(:department_entity) { DfE::Analytics.entities_for_analytics.find { |entity| entity.to_s.include?('department') } }
let(:entity_type) { 'entity_table_check' }
Copy link
Collaborator

Choose a reason for hiding this comment

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

Use instance variables in tests is normally discouraged. I would add a let for the checksum_calculated_at instead of using an instance variable in the tests.

  let(:checksum_calculated_at) { @checksum_calculated_at }

Then just use checksum_calculated_at rather than @checksum_calculated_at

@ericaporter ericaporter merged commit be1e516 into main Apr 23, 2024
7 checks passed
@ericaporter ericaporter deleted the fix-flakey-spec branch April 23, 2024 15:08
ericaporter added a commit that referenced this pull request Apr 23, 2024
* Freeze current_timestamp, set instance variable
* Update variable syntax
ericaporter added a commit that referenced this pull request Apr 23, 2024
* Freeze current_timestamp, set instance variable
* Update variable syntax
@ericaporter ericaporter mentioned this pull request Apr 24, 2024
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

2 participants