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 broken test case #81

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Conversation

fatkodima
Copy link
Member

$ rake test
Run options: --seed 64620

# Running:

.........................F

Failure:
ActionCacheTest#test_explicit_html_format_is_used_for_fragment_path [/Users/fatkodima/Desktop/oss/actionpack-action_caching/test/caching_test.rb:817]:
Expected: nil
  Actual: "1662930799.070181<div />"


rails test Users/fatkodima/Desktop/oss/actionpack-action_caching/test/caching_test.rb:803

............

Finished in 0.509232s, 74.6222 runs/s, 349.5460 assertions/s.
38 runs, 178 assertions, 1 failures, 0 errors, 0 skips
rake aborted!
Command failed with status (1)

This test case had a bug and correctly started to fail after the rails/rails#43735.

So before that Rails' PR:

  1. @cache_this instance variable variable was not reset after the first get in that test case
  2. after the second get, the action was not executed and content_to_cache returned a stale @cache_this and compared it to the cached response from the action

After the Rails' PR:

  1. @cache_this is reset to nil before the second get
  2. content_to_cache return nil from the step 1, but cached response is not equal to nil, so here is the test failure

Having cached_time = content_to_cache line is incorrect and this bug was unnoticed before that PR, so we need to remove it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

1 participant