Skip to content

Add testing tweaks#9

Merged
cheshire137 merged 4 commits into
add-testingfrom
add-testing-tweaks
Oct 3, 2017
Merged

Add testing tweaks#9
cheshire137 merged 4 commits into
add-testingfrom
add-testing-tweaks

Conversation

@MikeMcQuaid
Copy link
Copy Markdown
Contributor

Based on my comments on #4 and #5

Comment thread script/cibuild

./script/setup

set +e
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Why does this get run again after line 2?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

To allow not failing immediately if rake test fails but also outputting the rubocop failure output (if any)

Comment thread test/test_helper.rb
metadata = begin
YAML.load(parts[1])
begin
YAML.safe_load(parts[1])
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Today Rubocop Autocorrected For Me 😜

Comment thread test/topics_test.rb Outdated
lines = File.readlines(path)

assert lines.size > 0
assert !lines.empty?
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

What about refute lines.empty?? Or even refute_predicate lines, :empty? if refute_predicate exists here. 🤔

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

👍 to any of them

Comment thread test/topics_test.rb Outdated
refute_empty metadata, "expected some metadata for topic"

metadata.each do |key, value|
metadata.each_key do |key,|
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

You can drop the comma.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

👍

Comment thread test/topics_test.rb
assert metadata[key]&.strip&.size&.positive?,
"expected to have a value for '#{key}'"
end
end
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

The indentation looks wonky here.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Yeh, I may have added an extra end here.

@MikeMcQuaid
Copy link
Copy Markdown
Contributor Author

@cheshire137 Addressed your feedback, thanks!

@cheshire137 cheshire137 merged commit 6e51644 into add-testing Oct 3, 2017
@cheshire137 cheshire137 deleted the add-testing-tweaks branch October 3, 2017 16:55
@cheshire137
Copy link
Copy Markdown
Member

Aww, script/cibuild wants me to upgrade my ImageMagick! It fails for me, I'll try running the steps it suggests to fix it:

==> Pouring imagemagick-7.0.7-4.sierra.bottle.tar.gz
Error: The `brew link` step did not complete successfully
The formula built, but is not symlinked into /usr/local
Could not symlink bin/Magick++-config
Target /usr/local/bin/Magick++-config
is a symlink belonging to imagemagick@6. You can unlink it:

@cheshire137
Copy link
Copy Markdown
Member

brew link --overwrite imagemagick allowed script/cibuild to work for me locally. All good!

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.

3 participants