If I correctly recall – I haven’t looked yet, because Tozier isn’t here – we were correctly FTPing the category files. We were pretty whipped last Thursday, so there may be things to clean up, but I think it should be straightforward to move the index.html and feed.xml files, just more of the same. I’ll try to be strong enough to write tests even though the code is trivial and nothing could go wrong1.

  def test_feed_xml_is_uploaded
    jr = set_up_for_jekyll_testing
    jr.run
    sought = './_target/feed.xml'
    assert(File.exist?(sought), 
      "end to end can't find ftped #{sought}")  
  end

That seemed a reasonable test and this seems a reasonable implementation:

  def move_feed_xml(simulated)
    perform_in_folder("#{@jekyll_folder}/_site/") do 
      ftp_connection = ftp_connected_to_target unless simulated
      ftp_one_file(ftp_connection, 'feed.xml', simulated)
      ftp_connection.close unless simulated
    end
  end

And the test runs … but when we look at the output folder, feed.xml isn’t there. We watch the tests run and we see it flinging in and out. Some of our early tests move things after clearing the _target folder, but are not calling the full run method. We should consider cleaning out these early tests.

Also we note profound duplication. (As Tozier put it “We have learned that cut and paste is a good programming technique?”) Here’s the duplication we refer to:

  def move_the_articles(simulated)
    perform_in_folder("#{@jekyll_folder}/_site/articles") do 
      ftp_connection = ftp_connected_to_target unless simulated
      ftp_connection.chdir('articles') unless simulated
      result = files_to_ftp(@ipad).collect do |file|
        ftp_one_file(ftp_connection, file, simulated)
      end
      ftp_connection.close unless simulated
      result
    end
  end

  def move_the_categories(simulated)
    perform_in_folder("#{@jekyll_folder}/_site/categories") do 
      ftp_connection = ftp_connected_to_target unless simulated
      ftp_connection.chdir('categories') unless simulated
      result = 
    files_to_ftp('test_jekyll_site/_site/categories').collect do |file|
        ftp_one_file(ftp_connection, file, simulated)
      end
      ftp_connection.close unless simulated
      result
    end
  end

  def move_feed_xml(simulated)
    perform_in_folder("#{@jekyll_folder}/_site/") do 
      ftp_connection = ftp_connected_to_target unless simulated
      ftp_one_file(ftp_connection, 'feed.xml', simulated)
      ftp_connection.close unless simulated
    end
  end

There’s a lot of “same” up there. I’m not of a mind to fix that yet. But my pair is looking askance at me. I ask “Do you think we ought to fix this?” He mumbled. I asked again, in italics. He said “I think we ought to fix this”. Ever one to placate, I agree.

We decide to remove the duplication in the first two, and then find a way to use whatever comes out of that for the third, which is a bit different.

One irritating issue is that result variable. We’re dealing with that at present by calling the functions in “the right order” so that our test that looks at that list gets what it expects. We discuss removing that test. It was useful as a scaffold, telling us that if we turned FTP loose it was likely to do the right thing. But now we actually test whether it does the right thing. So we’ll remove this test:

  def test_what_will_be_ftped
    jr = set_up_for_jekyll_testing
    jr.move_ipad_files
    list_of_allegedly_moved_things = jr.ftp_the_files(simulated = true) 
    assert list_of_allegedly_moved_things.include? "/Users/ron/programming/test-ftp/test_jekyll_site/_site/articles/subfolder/index.html to something/subfolder/index.html"
  end

Now we can remove the result= stuff and the calculation of the result:

This:

  def ftp_one_file(ftp, file_name, simulated)
    unless simulated
      if File::directory? file_name
        ftp_mkdir_safely(ftp, file_name)
      else
        ftp.putbinaryfile(file_name,file_name)
      end
    end
    pwd = simulated ? "something" : ftp.pwd
    return "#{Dir.pwd}/#{file_name} to #{pwd}/#{file_name}"
  end

Loses its return statement and the result = and the returning come out of the other two methods:

  def move_the_articles(simulated)
    perform_in_folder("#{@jekyll_folder}/_site/articles") do 
      ftp_connection = ftp_connected_to_target unless simulated
      ftp_connection.chdir('articles') unless simulated
      files_to_ftp(@ipad).collect do |file|
        ftp_one_file(ftp_connection, file, simulated)
      end
      ftp_connection.close unless simulated
    end
  end

  def move_the_categories(simulated)
    perform_in_folder("#{@jekyll_folder}/_site/categories") do 
      ftp_connection = ftp_connected_to_target unless simulated
      ftp_connection.chdir('categories') unless simulated
      files_to_ftp('test_jekyll_site/_site/categories').collect do |file|
        ftp_one_file(ftp_connection, file, simulated)
      end
      ftp_connection.close unless simulated
    end
  end

We still have that .collect which is no longer needed, giving:

  def move_the_articles(simulated)
    perform_in_folder("#{@jekyll_folder}/_site/articles") do 
      ftp_connection = ftp_connected_to_target unless simulated
      ftp_connection.chdir('articles') unless simulated
      files_to_ftp(@ipad).each do |file|
        ftp_one_file(ftp_connection, file, simulated)
      end
      ftp_connection.close unless simulated
    end
  end

  def move_the_categories(simulated)
    perform_in_folder("#{@jekyll_folder}/_site/categories") do 
      ftp_connection = ftp_connected_to_target unless simulated
      ftp_connection.chdir('categories') unless simulated
      files_to_ftp('test_jekyll_site/_site/categories').each do |file|
        ftp_one_file(ftp_connection, file, simulated)
      end
      ftp_connection.close unless simulated
    end
  end

OK, now let’s extract a method. (Tozier flags whether we use simulated any more. Perhaps we could get rid of it.) Anyway, we quickly and fairly smoothly get to here:

  def move_batch(local_site_folder, ftp_target_folder, shipping_list_folder, simulated )
    perform_in_folder(local_site_folder) do 
      ftp_connection = ftp_connected_to_target unless simulated
      ftp_connection.chdir(ftp_target_folder) unless simulated
      files_to_ftp(shipping_list_folder).each do |file|
        ftp_one_file(ftp_connection, file, simulated)
      end
      ftp_connection.close unless simulated
    end
  end

  def move_the_articles(simulated)
    move_batch("#{@jekyll_folder}/_site/articles", 
      'articles', @ipad, simulated )
  end

  def move_the_categories(simulated)
    move_batch("#{@jekyll_folder}/_site/categories", 
      'categories', 'test_jekyll_site/_site/categories', simulated )
  end

Tozier observes that the name test_... shouldn’t be in our production code, so there is another parameter we need to figure out. Chet notices simulated, which we can probably just remove now. We’ll do a quick check.

We think we’ve deleted the only test that used the flag so we’ll remove it everwhere. Let’s paste the whole shebang as the changes were all over:

require "net/ftp"
require "./passwords"

class JekyllRunner
  def initialize(ipad_folder, jekyll_folder, host_name, host_folder, prefix)
    @ipad = ipad_folder
    @jekyll_folder = jekyll_folder
    @ftp_host_name = host_name
    @ftp_folder = host_folder
    @id_prefix = prefix
    @dir_pwd = Dir.pwd
  end

  def ftp_connected_to_target
    host = Object.const_get('Passwords::' + @id_prefix + 'USER')
    pass = Object.const_get('Passwords::' + @id_prefix + 'PASSWORD')
    ftp = Net::FTP.new(@ftp_host_name)
    ftp.login(host, pass)
    ftp.chdir(@ftp_folder)
    ftp
  end

  def move_ipad_files
    FileUtils.cp_r("#{@ipad}/.","#{@jekyll_folder}/articles")
  end

  def ftp_mkdir_safely(ftp, folder_string)
    ftp.mkdir(folder_string) unless ftp_folder_exists?(ftp, folder_string)
  end

  def ftp_folder_exists?(ftp, folder_string)
    split = folder_string.split("/")
    proposed_folder = split.last
    ppl = split[0...-1]
    proposed_prefix = "./" + ppl.join("/")
    ftp.list(proposed_prefix).any? { |name| name.match(proposed_folder) }
  end

  def perform_in_folder(new_dir, &block)
      pwd = Dir.pwd
      Dir.chdir(@dir_pwd)
      Dir.chdir(new_dir)
    begin
      result = block.call
    ensure
      Dir.chdir(pwd)
    end
    return result
  end

  def files_to_ftp(folder)
    perform_in_folder(folder) do
      Dir.glob('**/*').collect { |f| rename_md_to_html(f) }
    end
  end

  def ftp_the_files
    move_the_categories
    move_feed_xml
    move_the_articles
  end

  def move_batch(local_site_folder, ftp_target_folder, shipping_list_folder )
    perform_in_folder(local_site_folder) do 
      ftp_connection = ftp_connected_to_target
      ftp_connection.chdir(ftp_target_folder)
      files_to_ftp(shipping_list_folder).each do |file|
        ftp_one_file(ftp_connection, file)
      end
      ftp_connection.close
    end
  end

  def move_the_articles
    move_batch("#{@jekyll_folder}/_site/articles", 
      'articles', @ipad )
  end

  def move_the_categories
    move_batch("#{@jekyll_folder}/_site/categories", 
      'categories', 'test_jekyll_site/_site/categories' )
  end

  def move_feed_xml
    perform_in_folder("#{@jekyll_folder}/_site/") do 
      ftp_connection = ftp_connected_to_target
      ftp_one_file(ftp_connection, 'feed.xml')
      ftp_connection.close
    end
  end

  def ftp_one_file(ftp, file_name)
    if File::directory? file_name
      ftp_mkdir_safely(ftp, file_name)
    else
      ftp.putbinaryfile(file_name,file_name)
    end
  end

  def rename_md_to_html(file_name)
    file_name.gsub(/\.md$/,".html")
  end

  def run_jekyll
    perform_in_folder(@jekyll_folder) {`jekyll build`}
  end

  def run
    move_ipad_files
    run_jekyll
    ftp_the_files
  end
end

Let’s think about whether we can fold our feed.xm code into move_batch.

  def move_batch(local_site_folder, ftp_target_folder, shipping_list_folder )
    perform_in_folder(local_site_folder) do 
      ftp_connection = ftp_connected_to_target
      ftp_connection.chdir(ftp_target_folder)
      files_to_ftp(shipping_list_folder).each do |file|
        ftp_one_file(ftp_connection, file)
      end
      ftp_connection.close
    end
  end

  def move_the_articles
    move_batch("#{@jekyll_folder}/_site/articles", 
      'articles', @ipad )
  end

  def move_the_categories
    move_batch("#{@jekyll_folder}/_site/categories", 
      'categories', 'test_jekyll_site/_site/categories' )
  end

  def move_feed_xml
    perform_in_folder("#{@jekyll_folder}/_site/") do 
      ftp_connection = ftp_connected_to_target
      ftp_one_file(ftp_connection, 'feed.xml')
      ftp_connection.close
    end
  end

Tozier re-objects that we still have the test_... file name in there. This is surely evidence of something wrong. We think that this folder name came about by hammering until we were looking at the right place. However, what we mean is to use the _site/categories of the rendered Jekyll site. Don’t we already have that as one of our parameters?

I’m thinking we should change move_batch to accept a source folder, a destination FTP folder, and a list of files to move. This would cause us not to use files_to_ftp at all. Something like this:

  def move_batch(local_site_folder, ftp_target_folder, folders_and_files )
    perform_in_folder(local_site_folder) do 
      ftp_connection = ftp_connected_to_target
      ftp_connection.chdir(ftp_target_folder)
      folders_and_files.each do |file|
        ftp_one_file(ftp_connection, file)
      end
      ftp_connection.close
    end
  end

Now we need to make our two users provide their list explicitly. By the way, I don’t expect to be satisfied with how they do that. I do expect to have the problem moved to a better place.

  def move_batch(local_site_folder, ftp_target_folder, folders_and_files )
    perform_in_folder(local_site_folder) do 
      ftp_connection = ftp_connected_to_target
      ftp_connection.chdir(ftp_target_folder)
      folders_and_files.each do |file|
        ftp_one_file(ftp_connection, file)
      end
      ftp_connection.close
    end
  end

  def move_the_articles
    move_batch("#{@jekyll_folder}/_site/articles", 
      'articles', files_to_ftp(@ipad) )
  end

  def move_the_categories
    move_batch("#{@jekyll_folder}/_site/categories", 
      'categories', files_to_ftp('test_jekyll_site/_site/categories') )
  end

  def move_feed_xml
    perform_in_folder("#{@jekyll_folder}/_site/") do 
      ftp_connection = ftp_connected_to_target
      ftp_one_file(ftp_connection, 'feed.xml')
      ftp_connection.close
    end
  end

We think we can change this:

  def move_the_categories
    move_batch("#{@jekyll_folder}/_site/categories", 
      'categories', files_to_ftp('test_jekyll_site/_site/categories') )
  end

To this:

  def move_the_categories
    move_batch("#{@jekyll_folder}/_site/categories", 
      'categories', files_to_ftp("#{@jekyll_folder}/_site/categories") )
  end

This works, and it ties moving the categories back to the Jekyll folder, which makes sense and is one of our member variables. This seems nearly right.

Now let’s see if we can make our feed.xml function use move_batch.

  def move_feed_xml
    move_batch("#{@jekyll_folder}/_site/", '.', ['feed.xml'])
  end

This works as advertised, once we pass ‘.’ instead of ‘’. And, quoting Tozier, this “paves the way” for our final file:

  def test_index_html_is_uploaded
    jr = set_up_for_jekyll_testing
    jr.run
    sought = './_target/index.html'
    assert(File.exist?(sought), 
      "end to end can't find ftped #{sought}")  
  end

  def move_index_html
    move_batch("#{@jekyll_folder}/_site/", '.', ['index.html'])
  end

The tests run. We are now “confident” that we’re moving all the files we’ve thought of, all the way from the iPad source, through to the FTP site. Next time, we might be ready to move to the real site. Of course there are other features, such as the file watcher, that are needed as well.

Anyway, we’re calling this a good two hours’ work and we weren’t even very confused at any point. Next time we’ll review the code to see what it needs and then see what feature we need next.

  1. Famous last words. Perhaps unrelatedly, the tag I used for this footnote was fml, which I believe has another connotation which might also be applicable. Anyway, I plan to test. (Cf. “hold my beer”). ↩