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

Add measuring support #124

Merged
merged 9 commits into from
Feb 20, 2015
Merged

Conversation

norio-nomura
Copy link
Contributor

Support output from XCTestCase.measureBlock() as following notated image:

screenshot 2015-02-07 20 45 14

Support output from `XCTestCase.measureBlock()`
@@ -77,6 +79,10 @@ def format_pending_test(suite, test_case)
INDENT + format_test("#{test_case} [PENDING]", :pending)
end

def format_measuring_test(suite, test_case, time)
INDENT + format_test("#{test_case} measured (#{colored_time(time)} seconds)", :measure)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Line is too long. [93/80]

@norio-nomura
Copy link
Contributor Author

Should I rewrite codes following hound's review?

@supermarin
Copy link
Contributor

@norio-nomura i've deleted some of unneeded comments, and left some of them that are desireable to fix.

Thanks for your contribution, appreciate that!

@@ -27,6 +27,7 @@ def format_linking(file, build_variant, arch); EMPTY; end
def format_libtool(library); EMPTY; end
def format_passing_test(suite, test, time); EMPTY; end
def format_pending_test(suite, test); EMPTY; end
def format_measuring_test(suite, test, time); EMPTY; end
Copy link
Collaborator

Choose a reason for hiding this comment

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

Unused method argument - suite. If it's necessary, use _ or _suite as an argument name to indicate that it won't be used. You can also write as format_measuring_test(*) if you want the method to accept any arguments but don't care about them.
Unused method argument - test. If it's necessary, use _ or _test as an argument name to indicate that it won't be used. You can also write as format_measuring_test(*) if you want the method to accept any arguments but don't care about them.
Unused method argument - time. If it's necessary, use _ or _time as an argument name to indicate that it won't be used. You can also write as format_measuring_test(*) if you want the method to accept any arguments but don't care about them.
Use empty lines between defs.
Unnecessary spacing detected.

@supermarin
Copy link
Contributor

@norio-nomura thanks for addressing the code style guide.
It seems like the parameters got misaligned on some of these cases - it'd be awesome if you could align them before merging

@@ -180,6 +180,13 @@ module XCPretty
@parser.parse(SAMPLE_PENDING_KIWI_TEST)
end

it "parses measuring tests" do
@formatter.should receive(:format_measuring_test).with('SecEncodeTransformTests.SecEncodeTransformTests',
'test_RFC4648_Decode_UsingBase32',
Copy link
Collaborator

Choose a reason for hiding this comment

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

Line is too long. [95/80]

@norio-nomura
Copy link
Contributor Author

How about the symbol "◷" for MEASURE?

@norio-nomura
Copy link
Contributor Author

Example of "◷" used on Travis-CI will be following:
https://travis-ci.org/norio-nomura/Base32/builds/51453875

Line is too long.
Prefer single-quoted strings.
@supermarin
Copy link
Contributor

@norio-nomura thanks, that looks actually good to me.
I'd wait a bit to see if @kattrali is around, if not feel free to commit it.

Other than that, the code is good to merge.
We'll need to add a feature (a frontend test), but if you feel ambitious, it'd be greatly appreciated.

@norio-nomura
Copy link
Contributor Author

Oh, sorry, I didn't read about feature.
I will read Cucumber documents.

@kattrali
Copy link
Contributor

@supermarin @norio-nomura "◷" sounds perfect to me

@@ -181,6 +185,14 @@
run_output.should start_with(yellow("P"))
end

Then(/^I should see a measuring test icon in ASCII$/) do
run_output.should start_with("T")
Copy link
Collaborator

Choose a reason for hiding this comment

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

Prefer single-quoted strings when you don't need string interpolation or special symbols.

end

Then(/^I should see a yellow measuring test icon$/) do
run_output.should start_with(yellow("T"))
Copy link
Collaborator

Choose a reason for hiding this comment

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

Prefer single-quoted strings when you don't need string interpolation or special symbols.

def format_measuring_test(suite, test_case, time)
yellow(MEASURING)
end

Copy link
Collaborator

Choose a reason for hiding this comment

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

Extra empty line detected at class body end.

Prefer single-quoted strings.
Prefer single-quoted strings.
@norio-nomura
Copy link
Contributor Author

It's my first writing feature.
I copy and modify the PENDING cases.
It's interest.

@kattrali
Copy link
Contributor

@norio-nomura 👍 😄

@supermarin
Copy link
Contributor

@norio-nomura you're awesome!

supermarin added a commit that referenced this pull request Feb 20, 2015
@supermarin supermarin merged commit 3e2ba4c into xcpretty:master Feb 20, 2015
@norio-nomura
Copy link
Contributor Author

@supermarin @kattrali Thanks! 😄

@norio-nomura norio-nomura deleted the add-measuring branch February 21, 2015 11:32
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.

None yet

4 participants