8000 Test against Jekyll 3 by pathawks · Pull Request #72 · jekyll/jekyll-redirect-from · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

Test against Jekyll 3 #72

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
Merged
merged 2 commits into from
Sep 1, 2015
Merged

Test against Jekyll 3 #72

merged 2 commits into from
Sep 1, 2015

Conversation

pathawks
Copy link
Member

@pathawks
Copy link
Member Author
8000 pathawks commented Aug 31, 2015

The test chokes when testing with GitHub Pages:

Failures:
  1) JekyllRedirectFrom::RedirectPage when determining the write destination of a redirect page meant to be a file knows not to add the index.html if it's not a folder
     Failure/Error: expect(redirect_page.destination("/")).to eql(dest)

       expected: "/home/travis/build/jekyll/jekyll-redirect-from/spec/fixtures/_site/posts/12435151125/larry-had-a-little-lamb"
            got: "/posts/12435151125/larry-had-a-little-lamb"

       (compared using eql?)
     # ./spec/jekyll_redirect_from/redirect_page_spec.rb:45:in `block (4 levels) in <top (required)>'
  2) JekyllRedirectFrom::RedirectPage when determining the write destination of a redirect page meant to be a dir knows to add the index.html if it's a folder
     Failure/Error: expect(redirect_page.destination("/")).to eql(dest)

       expected: "/home/travis/build/jekyll/jekyll-redirect-from/spec/fixtures/_site/posts/1914798137981389/larry-had-a-little-lamb/index.html"
            got: "/posts/1914798137981389/larry-had-a-little-lamb/index.html"

       (compared using eql?)
     # ./spec/jekyll_redirect_from/redirect_page_spec.rb:38:in `block (4 levels) in <top (required)>'
Finished in 1.29 seconds (files took 0.31592 seconds to load)
31 examples, 2 failures

Clearly the test is wrong. Any ideas why?

@parkr
Copy link
Member
parkr commented Aug 31, 2015

Comparing paths is always annoying. It looks like #destination requires the absolute path to dest, perhaps, instead of just "/"?

@pathawks
Copy link
Member Author

Does it has something to do with safe mode or file system quarantine?

It only fails when testing with the GitHub Pages gem. In all other cases, new_redirect_page(permalink) returns the absolute path, but when testing with gh-pages it returns a relative path.

@parkr
Copy link
Member
parkr commented Aug 31, 2015

It only fails when testing with the GitHub Pages gem.

Perhaps it is a failure of Jekyll. I bet we fixed something in 2.5 that GHP's 2.4 isn't getting.

@pathawks
Copy link
Member Author
pathawks commented Sep 1, 2015

Perhaps it is a failure of Jekyll. I bet we fixed something in 2.5 that GHP's 2.4 isn't getting.

Yes, I can confirm that the test fails with 2.4 and passes with 2.5.0.

So... maybe shouldn't require the gh-pages test to pass for now?

parkr added a commit that referenced this pull request Sep 1, 2015
@parkr
Merge pull request #72 from pathawks/Jekyll3
0c740f6 
Merge pull request 72
@parkr parkr merged commit 0c740f6 into jekyll:master Sep 1, 2015
parkr added a commit that referenced this pull request Sep 1, 2015
@parkr
Update history to reflect merge of #72
5ae615a 
[ci skip]
@parkr
Copy link
Member
parkr commented Sep 1, 2015

Thank you! You're killin it. I appreciate all these PR's.

@pathawks pathawks deleted the Jekyll3 branch September 1, 2015 14:27
@pathawks
Copy link
Member Author
pathawks commented Sep 1, 2015

Happy to help in the small ways I am able 👍

@jekyll jekyll locked and limited conversation to collaborators Feb 27, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers
No reviews
Assignees
No one assigned
Labels
frozen-due-to-age
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@pathawks @parkr @jekyllbot
0