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.

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

Functional JUnit reporter #1454

Merged
merged 4 commits into from
Feb 27, 2017
Merged

Functional JUnit reporter #1454

merged 4 commits into from
Feb 27, 2017

Conversation

jkerry
Copy link
Contributor

@jkerry jkerry commented Feb 3, 2017 •
edited
Loading

resolves #1438 by removing the gem based junit reporter and manually rolling up JUnit formatted XML based on the JSON reporter's @output_hash.

Some minor ancillary changes:

  • The testsuite is no longer expected to have an errors count
  • nokogiri gem elevated to a core dependency

@jkerry jkerry force-pushed the FunctionalJUnitReporter branch from 75d375c to 7cabd42 Compare February 3, 2017 04:22
@jkerry
Copy link
Contributor Author

jkerry commented Feb 3, 2017

Is there any SOP for digging deeper here on the travis build? not seeing the same failure when running functional tests on the macbook

@jkerry jkerry closed this Feb 3, 2017
@jkerry jkerry reopened this Feb 3, 2017
@jkerry
Copy link
Contributor Author

jkerry commented Feb 3, 2017

no change, rerun, different failure 👎

@jkerry jkerry closed this Feb 3, 2017
@jkerry jkerry reopened this Feb 3, 2017
@jkerry
Copy link
Contributor Author

jkerry commented Feb 3, 2017

image

chris-rock
chris-rock approved these changes Feb 23, 2017
View reviewed changes
Copy link
Contributor

@chris-rock chris-rock left a comment

Choose a reason for hiding this comment

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

Awesome improvement @jkerry Could you please rebase on latest master?

lib/inspec/rspec_json_formatter.rb
@@ -748,13 +748,73 @@ def status_type(example)
end
end

class InspecRspecJUnit < RSpecJUnitFormatter
class InspecRspecJUnit < InspecRspecJson
Copy link
Contributor

Choose a reason for hiding this comment

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

It is so good to see that this is now based on our InSpec JSON generator. Awesome @jkerry

@chris-rock chris-rock requested a review from adamleff February 23, 2017 16:29
@chris-rock chris-rock added the Type: Enhancement Improves an existing feature label Feb 23, 2017
adamleff
adamleff approved these changes Feb 23, 2017
View reviewed changes
Copy link
Contributor

@adamleff adamleff left a comment

Choose a reason for hiding this comment

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

Small misspelling. otherwise, approved :)

lib/inspec/rspec_json_formatter.rb Outdated
end

def build_result_xml(xml, control, result)
test_class = control[:title].nil? ? 'Anonymnous' : control[:id]
Copy link
Contributor

Choose a reason for hiding this comment

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

"Anonymous" is misspelled here.

@jkerry jkerry force-pushed the FunctionalJUnitReporter branch from 7cabd42 to 8a5ccdc Compare February 24, 2017 15:32
@jkerry
Copy link
Contributor Author

jkerry commented Feb 24, 2017

Added! one peculiarity I noticed while developing this is if a case is explicitly skipped with a guard it won't show up in the json output as 'skipped'. It is simply omitted from the output. Is that a feature or a bug? A case that describes, say, a file that isn't present shows in the output as skipped.

@jkerry
Copy link
Contributor Author

jkerry commented Feb 24, 2017

Doh, rebase busted the tests. I'll get this handled today

@chris-rock
Copy link
Contributor

@jkerry That may be related to #1515

@adamleff
Copy link
Contributor

@chris-rock I don't believe it's related to the rb-readline issues because we'd be seeing a bunch of inspec_shell_test failures here too. These look like legitimate test failures, especially this one:

  2) Error:
inspec exec with junit formatter::execute a profile with junit formatting::the test suite::the testcase named "gordon_config Can't find file ..."#test_0002_should be skipped:
NoMethodError: undefined method `xpath' for nil:NilClass
    /home/travis/build/chef/inspec/test/functional/inspec_exec_junit_test.rb:71:in `block (5 levels) in <top (required)>'

@jkerry jkerry force-pushed the FunctionalJUnitReporter branch from 8a5ccdc to e0ae7eb Compare February 25, 2017 05:44
@jkerry
Copy link
Contributor Author

jkerry commented Feb 25, 2017

Yeah, this was on me. I ran a rebase and force push off of a stale local repo. I can't run functional tests on a windows development box so the real source of truth was on the macbook.

@jkerry
Copy link
Contributor Author

jkerry commented Feb 27, 2017

@chris-rock @adamleff technically this was a history rewrite so you might want to revisit the code review just in case

@chris-rock
Copy link
Contributor

Thank you for the additional effort @jkerry

@adamleff
Copy link
Contributor

Thank you, @jkerry - this looks great. We greatly appreciate your contribution!

@adamleff adamleff merged commit 19f114d into inspec:master Feb 27, 2017
@ryancurrah
Copy link

Woot! @jkerry thanks so much!

@jkerry jkerry deleted the FunctionalJUnitReporter branch February 27, 2017 18:48
adamleff added a commit that referenced this pull request Apr 3, 2017
@adamleff
Replace Nokogiri with REXML in the JUnit formatter
73d46f9 
In #1454, we welcomed a newly-revamped JUnit formatter which has
a dependency on Nokogiri. Unfortunately, this had led us to problems
getting InSpec included in Chef omnibus builds (see chef/chef#5937)
because Chef is using Ruby 2.4.1 and the Nokogiri maintainers have
not yet released a windows binary gem that supports Ruby 2.4.x.
This has led to breaking builds in Chef's CI platform and would
block the acceptance of chef/chef#5937.

This change replaces Nokogiri use with REXML instead. While REXML
can be slower than Nokogiri, it does not require native extensions
and is supported on all Chef platforms.

Signed-off-by: Adam Leff <[email protected]>
@adamleff adamleff mentioned this pull request Apr 3, 2017
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@chris-rock chris-rock chris-rock approved these changes

@adamleff adamleff adamleff approved these changes

Assignees
No one assigned
Labels
Type: Enhancement Improves an existing feature
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

jUnit reports are hard to read
4 participants
@jkerry @chris-rock @adamleff @ryancurrah