From 25206fed4035e638a789cab4f1c44a74efc4870d Mon Sep 17 00:00:00 2001 From: Jeremie Veillet Date: Wed, 3 Apr 2019 17:19:29 +0200 Subject: [PATCH 1/4] Avoid overwriting an existing robots.txt --- lib/jekyll/jekyll-sitemap.rb | 6 +----- spec/jekyll-sitemap_spec.rb | 28 +++++++++++++++++++++++++++- 2 files changed, 28 insertions(+), 6 deletions(-) diff --git a/lib/jekyll/jekyll-sitemap.rb b/lib/jekyll/jekyll-sitemap.rb index 448714c..dcd0de2 100644 --- a/lib/jekyll/jekyll-sitemap.rb +++ b/lib/jekyll/jekyll-sitemap.rb @@ -62,11 +62,7 @@ def robots # Checks if a file already exists in the site source def file_exists?(file_path) - if @site.respond_to?(:in_source_dir) - File.exist? @site.in_source_dir(file_path) - else - File.exist? Jekyll.sanitized_path(@site.source, file_path) - end + (@site.pages + @site.static_files).any? { |p| p.url == "/#{file_path}" } end end end diff --git a/spec/jekyll-sitemap_spec.rb b/spec/jekyll-sitemap_spec.rb index df543bc..c937750 100644 --- a/spec/jekyll-sitemap_spec.rb +++ b/spec/jekyll-sitemap_spec.rb @@ -195,13 +195,39 @@ expect(contents).not_to match(%r!\ATHIS IS MY LAYOUT!) end - it "creates a sitemap.xml file" do + it "creates a robots.txt file" do expect(File.exist?(dest_dir("robots.txt"))).to be_truthy end it "renders liquid" do expect(contents).to match("Sitemap: http://xn--mlaut-jva.example.org/sitemap.xml") end + + context "user defined robots.txt" do + before do + File.open("#{SOURCE_DIR}/robots.txt", "w") { |f| f.write("Allow: /") } + site.process + end + + let(:contents) { File.read(dest_dir("robots.txt")) } + + it "has no layout" do + expect(contents).not_to match(%r!\ATHIS IS MY LAYOUT!) + end + + it "creates a robots.txt file" do + expect(File.exist?(dest_dir("robots.txt"))).to be_truthy + end + + it "does not override user defined robots.txt" do + expect(contents).to match("Allow: /") + end + + after do + File.delete("#{SOURCE_DIR}/robots.txt") + site.process + end + end end end end From f4c24e062ff2a39ba2ae9477fb29aeae6a60a2f2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=A9r=C3=A9mie=20Veillet?= Date: Fri, 12 Apr 2019 13:46:18 +0200 Subject: [PATCH 2/4] Fix tests with a different destination directory --- .gitignore | 1 + spec/fixtures/user-defined-robots/robots.txt | 1 + spec/jekyll-sitemap_spec.rb | 43 ++++++++++---------- spec/spec_helper.rb | 5 +++ 4 files changed, 29 insertions(+), 21 deletions(-) create mode 100644 spec/fixtures/user-defined-robots/robots.txt diff --git a/.gitignore b/.gitignore index b2c75e5..5707394 100644 --- a/.gitignore +++ b/.gitignore @@ -1,5 +1,6 @@ *.gem Gemfile.lock spec/dest +spec/_site .bundle spec/fixtures/.jekyll-cache diff --git a/spec/fixtures/user-defined-robots/robots.txt b/spec/fixtures/user-defined-robots/robots.txt new file mode 100644 index 0000000..e9f7c88 --- /dev/null +++ b/spec/fixtures/user-defined-robots/robots.txt @@ -0,0 +1 @@ +Allow: / diff --git a/spec/jekyll-sitemap_spec.rb b/spec/jekyll-sitemap_spec.rb index c937750..d9ff8d0 100644 --- a/spec/jekyll-sitemap_spec.rb +++ b/spec/jekyll-sitemap_spec.rb @@ -202,31 +202,32 @@ it "renders liquid" do expect(contents).to match("Sitemap: http://xn--mlaut-jva.example.org/sitemap.xml") end + end - context "user defined robots.txt" do - before do - File.open("#{SOURCE_DIR}/robots.txt", "w") { |f| f.write("Allow: /") } - site.process - end - - let(:contents) { File.read(dest_dir("robots.txt")) } - - it "has no layout" do - expect(contents).not_to match(%r!\ATHIS IS MY LAYOUT!) - end + context "user defined robots.txt" do + let(:overrides) do + { + "source" => source_dir("/user-defined-robots/"), + "destination" => site_dest_dir, + "url" => "http://example.org", + "collections" => { + "my_collection" => { "output" => true }, + "other_things" => { "output" => false }, + }, + } + end + let(:contents) { File.read(site_dest_dir("robots.txt")) } - it "creates a robots.txt file" do - expect(File.exist?(dest_dir("robots.txt"))).to be_truthy - end + it "has no layout" do + expect(contents).not_to match(%r!\ATHIS IS MY LAYOUT!) + end - it "does not override user defined robots.txt" do - expect(contents).to match("Allow: /") - end + it "creates a robots.txt file" do + expect(File.exist?(dest_dir("robots.txt"))).to be_truthy + end - after do - File.delete("#{SOURCE_DIR}/robots.txt") - site.process - end + it "does not override user defined robots.txt" do + expect(contents).to match("Allow: /") end end end diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index 911bc45..3c47445 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -12,6 +12,7 @@ SOURCE_DIR = File.expand_path("fixtures", __dir__) DEST_DIR = File.expand_path("dest", __dir__) + SITE_DIR = File.expand_path("_site", __dir__) def source_dir(*files) File.join(SOURCE_DIR, *files) @@ -20,4 +21,8 @@ def source_dir(*files) def dest_dir(*files) File.join(DEST_DIR, *files) end + + def site_dest_dir(*files) + File.join(SITE_DIR, *files) + end end From 4bf086c1125cc9ab1dcc2506cbb9f9b45340ed12 Mon Sep 17 00:00:00 2001 From: Ashwin Maroli Date: Thu, 25 Apr 2019 09:32:07 +0530 Subject: [PATCH 3/4] Refactor tests --- .gitignore | 1 - spec/jekyll-sitemap_spec.rb | 58 +++++++++++++------ spec/robot-fixtures/page-at-root/robots.txt | 4 ++ .../assets/robots.txt | 5 ++ .../static-at-source-root}/robots.txt | 0 .../static-in-subdir/assets/robots.txt | 1 + spec/spec_helper.rb | 18 +++++- 7 files changed, 65 insertions(+), 22 deletions(-) create mode 100644 spec/robot-fixtures/page-at-root/robots.txt create mode 100644 spec/robot-fixtures/permalinked-page-in-subdir/assets/robots.txt rename spec/{fixtures/user-defined-robots => robot-fixtures/static-at-source-root}/robots.txt (100%) create mode 100644 spec/robot-fixtures/static-in-subdir/assets/robots.txt diff --git a/.gitignore b/.gitignore index 5707394..b2c75e5 100644 --- a/.gitignore +++ b/.gitignore @@ -1,6 +1,5 @@ *.gem Gemfile.lock spec/dest -spec/_site .bundle spec/fixtures/.jekyll-cache diff --git a/spec/jekyll-sitemap_spec.rb b/spec/jekyll-sitemap_spec.rb index d9ff8d0..2b7d9c5 100644 --- a/spec/jekyll-sitemap_spec.rb +++ b/spec/jekyll-sitemap_spec.rb @@ -203,31 +203,53 @@ expect(contents).to match("Sitemap: http://xn--mlaut-jva.example.org/sitemap.xml") end end + end + + context "with user-defined robots.txt" do + let(:fixture) { "/" } + let(:fixture_source) { robot_fixtures(fixture) } + let(:fixture_dest) { robot_fixtures(fixture, "_site") } + let(:robot_contents) { File.read(robot_fixtures(fixture, "_site", "robots.txt")).strip } + let(:overrides) do + { + "source" => fixture_source, + "destination" => fixture_dest, + "url" => "http://example.org", + } + end + + before(:each) { setup_fixture(fixture) } + after(:each) { cleanup_fixture(fixture) } - context "user defined robots.txt" do - let(:overrides) do - { - "source" => source_dir("/user-defined-robots/"), - "destination" => site_dest_dir, - "url" => "http://example.org", - "collections" => { - "my_collection" => { "output" => true }, - "other_things" => { "output" => false }, - }, - } + context "as a static-file at source-root" do + let(:fixture) { "static-at-source-root" } + + it "doesn't override the robots file" do + expect(robot_contents).to eql("Allow: /") end - let(:contents) { File.read(site_dest_dir("robots.txt")) } + end - it "has no layout" do - expect(contents).not_to match(%r!\ATHIS IS MY LAYOUT!) + context "as a static-file in a subdir" do + let(:fixture) { "static-in-subdir" } + + it "generates a valid robot.txt" do + expect(robot_contents).to eql("Sitemap: http://example.org/sitemap.xml") end + end - it "creates a robots.txt file" do - expect(File.exist?(dest_dir("robots.txt"))).to be_truthy + context "as a page at root" do + let(:fixture) { "page-at-root" } + + it "doesn't override the robots file" do + expect(robot_contents).to eql("Allow: http://example.org") end + end + + context "as a page with permalink in a subdir" do + let(:fixture) { "permalinked-page-in-subdir" } - it "does not override user defined robots.txt" do - expect(contents).to match("Allow: /") + it "doesn't override the robots file" do + expect(robot_contents).to eql("Allow: http://example.org") end end end diff --git a/spec/robot-fixtures/page-at-root/robots.txt b/spec/robot-fixtures/page-at-root/robots.txt new file mode 100644 index 0000000..e63befd --- /dev/null +++ b/spec/robot-fixtures/page-at-root/robots.txt @@ -0,0 +1,4 @@ +--- +--- + +Allow: {{ site.url }} diff --git a/spec/robot-fixtures/permalinked-page-in-subdir/assets/robots.txt b/spec/robot-fixtures/permalinked-page-in-subdir/assets/robots.txt new file mode 100644 index 0000000..0e396d1 --- /dev/null +++ b/spec/robot-fixtures/permalinked-page-in-subdir/assets/robots.txt @@ -0,0 +1,5 @@ +--- +permalink: '/robots.txt' +--- + +Allow: {{ site.url }} diff --git a/spec/fixtures/user-defined-robots/robots.txt b/spec/robot-fixtures/static-at-source-root/robots.txt similarity index 100% rename from spec/fixtures/user-defined-robots/robots.txt rename to spec/robot-fixtures/static-at-source-root/robots.txt diff --git a/spec/robot-fixtures/static-in-subdir/assets/robots.txt b/spec/robot-fixtures/static-in-subdir/assets/robots.txt new file mode 100644 index 0000000..e9f7c88 --- /dev/null +++ b/spec/robot-fixtures/static-in-subdir/assets/robots.txt @@ -0,0 +1 @@ +Allow: / diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index 3c47445..ebeaef9 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -1,6 +1,7 @@ # frozen_string_literal: true require "jekyll" +require "fileutils" require File.expand_path("../lib/jekyll-sitemap", __dir__) Jekyll.logger.log_level = :error @@ -12,7 +13,8 @@ SOURCE_DIR = File.expand_path("fixtures", __dir__) DEST_DIR = File.expand_path("dest", __dir__) - SITE_DIR = File.expand_path("_site", __dir__) + ROBOT_FIXTURES = File.expand_path("robot-fixtures", __dir__) + ROBOT_FIXTURE_ITEMS = %w(_posts _layouts _config.yml index.html).freeze def source_dir(*files) File.join(SOURCE_DIR, *files) @@ -22,7 +24,17 @@ def dest_dir(*files) File.join(DEST_DIR, *files) end - def site_dest_dir(*files) - File.join(SITE_DIR, *files) + def robot_fixtures(*subdirs) + File.join(ROBOT_FIXTURES, *subdirs) + end + + def setup_fixture(directory) + ROBOT_FIXTURE_ITEMS.each { |item| FileUtils.cp_r(source_dir(item), robot_fixtures(directory)) } + end + + def cleanup_fixture(directory, dest_dirname = "_site") + (ROBOT_FIXTURE_ITEMS + [dest_dirname]).each do |item| + FileUtils.remove_entry(robot_fixtures(directory, item)) + end end end From d9d8d0d3f4bac585232d3aebf93b8ec28d2a7988 Mon Sep 17 00:00:00 2001 From: Ashwin Maroli Date: Thu, 25 Apr 2019 09:32:15 +0530 Subject: [PATCH 4/4] avoid allocating multiple arrays --- lib/jekyll/jekyll-sitemap.rb | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/lib/jekyll/jekyll-sitemap.rb b/lib/jekyll/jekyll-sitemap.rb index dcd0de2..54ff823 100644 --- a/lib/jekyll/jekyll-sitemap.rb +++ b/lib/jekyll/jekyll-sitemap.rb @@ -62,7 +62,11 @@ def robots # Checks if a file already exists in the site source def file_exists?(file_path) - (@site.pages + @site.static_files).any? { |p| p.url == "/#{file_path}" } + pages_and_files.any? { |p| p.url == "/#{file_path}" } + end + + def pages_and_files + @pages_and_files ||= @site.pages + @site.static_files end end end