Skip to content

Commit

Permalink
cpp_builder: remove workaround of Boost 1.62 bug, drop JUnit support
Browse files Browse the repository at this point in the history
This drops support for Boost 1.62, but I don't see much value in
supporting this broken version specifically, given that none of the
`cpp_stl` targets use it at the moment. As of March 2024, we're using
these versions:

* Boost 1.54.0 - in `{clang3.4,gcc4.8}-linux-x86_64` targets, since they
  both run on Ubuntu 14.04 (see
  https://github.com/kaitai-io/kaitai_struct_docker_images/blob/ef0ad6e3/src/cpp_stl/gcc4.8-linux-x86_64/Dockerfile#L1),
  which comes with this version of Boost:
  https://launchpad.net/ubuntu/trusty/+source/boost-defaults
* Boost 1.71.0 - in `msvc141-windows-x64`
  (https://ci.appveyor.com/project/kaitai-io/ci-targets/builds/49319128/job/282j67vaxhklm2o4?fullLog=true#L120)
* Boost 1.74.0 - in `{clang11,gcc11}-linux-x86_64`, since they both run
  on Ubuntu 22.04 (see
  https://github.com/kaitai-io/kaitai_struct_docker_images/blob/ef0ad6e3/src/cpp_stl/gcc11-linux-x86_64/Dockerfile#L1),
  which comes with this version of Boost:
  https://launchpad.net/ubuntu/jammy/+source/boost-defaults
* Boost 1.84.0 - in `clang14-macos-x86_64`

On top of that, the documented workaround of the bug (`--log_sink`
broken in Boost v1.62) in the code inadverently changed the log format
to JUnit (`--logger=JUNIT,...`), which is apparently why additional code
had to be added in 5d2125c to support
JUnit test result format in addition to the Boost-specific XML format.

This part is definitely unnecessary - there's no reason why we should
support two log formats, we should choose one. Even in Boost 1.62 the
XML format was definitely available as well, though it would have to be
specified as `--logger=XML,...` instead of `--log_format=XML` (see
https://web.archive.org/web/20221205203459/https://svn.boost.org/trac10/ticket/12507).
It looks like the reason the workaround uses JUnit is that the
`--logger=JUNIT,...` option was copied from the [Stack Overflow
answer](https://stackoverflow.com/a/39999085/487064) mentioned in the
code comment without thinking.

Fun fact: apparently, there hasn't been a single run using Boost 1.62
since December 2018, because there isn't any occurrence of `<testcase`
(typical for JUnit format) in the
https://github.com/kaitai-io/ci_artifacts repo under
`test_out/cpp_stl{_98,_11}/`. You can check this by cloning
https://github.com/kaitai-io/ci_artifacts locally and running this
command:

```
git log --stat -G '<testcase' --all -- test_out/cpp_stl{_98,_11}/
```

This yields no results, meaning that this workaround predates the
migration of C++ to the "new" modular CI at ci.kaitai.io
(kaitai-io/kaitai_struct#62) and has never
been used since then.
  • Loading branch information
generalmimon committed Mar 18, 2024
1 parent faaeb6a commit d07deb4
Show file tree
Hide file tree
Showing 2 changed files with 1 addition and 66 deletions.
47 changes: 0 additions & 47 deletions aggregate/boost_test_parser.rb
Original file line number Diff line number Diff line change
Expand Up @@ -8,14 +8,6 @@ def initialize(fn)
end

def each_test
# Boost actually uses two slightly different formats: old and
# new. Instead of trying to detect which one we have, we actually
# just try both.
each_from_old_boost_format { |tr| yield tr }
each_from_new_boost_format { |tr| yield tr }
end

def each_from_old_boost_format
@doc.root.elements.each('TestSuite') { |ts|
ts.elements.each('TestCase') { |tc|
name = tc.attribute('name').value
Expand Down Expand Up @@ -54,43 +46,4 @@ def each_from_old_boost_format
}
}
end

def each_from_new_boost_format
@doc.root.elements.each('testcase') { |tc|
name = tc.attribute('name').value

raise "Unable to parse name: \"#{name}\"" unless name =~ /^test_(.*?)$/
name = underscore_to_ucamelcase($1)

failures = []
tc.elements.each('failure') { |err|
failures << TestResult::Failure.new(
nil, # filename
nil, # line
err.text,
nil
)
}

tc.elements.each('error') { |err|
failures << TestResult::Failure.new(
nil, # filename
nil, # line
err.text,
nil
)
}

if failures.empty?
status = :passed
failure = nil
else
status = :failed
failure = failures[0]
end

tr = TestResult.new(name, status, tc.attribute('time').value.to_f, failure)
yield tr
}
end
end
20 changes: 1 addition & 19 deletions builder/cpp_builder.rb
Original file line number Diff line number Diff line change
Expand Up @@ -306,24 +306,6 @@ def file_to_test(filename)
def run_tests
xml_log = "#{@abs_cpp_test_out_dir}/results.xml"

# Work around boost v1.62 bug: https://svn.boost.org/trac10/ticket/12507
# --log_sink is broken in boost v1.62, using workaround, as per
# https://stackoverflow.com/a/39999085/487064
#
# However, Travis has boost v1.54, which has problems with it, so we
# won't use the workaround on anything except exactly v1.62

boost_log_option = "--log_sink=#{xml_log}"

begin
if File.read('/usr/include/boost/version.hpp') =~ /BOOST_VERSION 106200/
# Boost v1.62 detected, enabling workaround
boost_log_option = "--logger=JUNIT,test_suite,#{xml_log}"
end
rescue Errno::ENOENT => e
# ignore
end

# Choose test executable
case @mode
when :msbuild_windows
Expand All @@ -339,7 +321,7 @@ def run_tests
tests_cli = [
ks_tests_bin,
'--log_format=XML',
boost_log_option,
"--log_sink=#{xml_log}",
'--log_level=all',
'--report_level=detailed',
]
Expand Down

0 comments on commit d07deb4

Please sign in to comment.