Skip to content

Conversation

@Watson1978
Copy link
Contributor

@Watson1978 Watson1978 commented Jan 7, 2026

Which issue(s) this PR fixes:
Fixes #

What this PR does / why we need it:
This PR will add missing yaml parser tests.

Docs Changes:

Release Note:

@Watson1978 Watson1978 force-pushed the add-yaml-parser-test branch 5 times, most recently from 6b2ede6 to 7575ad1 Compare January 9, 2026 04:33
Signed-off-by: Shizuo Fujita <fujita@clear-code.com>
@Watson1978 Watson1978 force-pushed the add-yaml-parser-test branch from 7575ad1 to 6399577 Compare January 9, 2026 04:50
@Watson1978 Watson1978 marked this pull request as ready for review January 9, 2026 05:16
@Watson1978 Watson1978 requested a review from kenhys January 9, 2026 05:17
@Watson1978 Watson1978 added the backport to v1.19 We will backport this fix to the LTS branch label Jan 9, 2026
@Watson1978 Watson1978 added this to the v1.20.0 milestone Jan 9, 2026
File.open(path, "w:#{encoding}:utf-8") {|f| f.write data }
end

def test_special_yaml_elements_dollar
Copy link
Contributor

Choose a reason for hiding this comment

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

It might be better to group with sub_test-case

assert_equal("match", config.elements[2].name)
assert_equal("worker", config.elements[3].name)
assert_equal("label", config.elements[4].name)
assert_equal("filter", config.elements[5].name) # included section
Copy link
Contributor

Choose a reason for hiding this comment

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

It could be simplified with:

assert_equal(%w(source filter match worker label filter), config.elements.map(&:name))

config = Fluent::Config::YamlParser.parse("#{TMP_DIR}/test_merge_common_parameter_using_include.yaml")
assert_equal(2, config.elements.size)
assert_equal('foobarbaz', config.elements[0].elements[0]['common_param'])
assert_equal('foobarbaz', config.elements[1].elements[0]['common_param'])
Copy link
Contributor

Choose a reason for hiding this comment

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

elements[X].elements[Y]で参照するのはおそらく意図したものではなさそう。
現状パースエラーになるけど、以下をincludeするものとしたテストコードであるべきじゃないだろうか。

https://docs.fluentd.org/configuration/config-file-yaml#share-the-same-parameters

write_config "#{TMP_DIR}/fluent-common.yaml", <<~EOS
      common_param: foobarbaz
EOS

Copy link
Contributor

@kenhys kenhys Jan 9, 2026

Choose a reason for hiding this comment

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

たとえば、テストケースのconfig.elements[0]は以下のように現状ではパースされる。

<name:match, arg:dummy_tag_1, attrs:{...}, elements:[<name:<<, arg:, attrs:{"common_param" => "foobarbaz"}, elements:[]>]>

config.elements[0].elements[0].name == '<<'になってしまっている。

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

backport to v1.19 We will backport this fix to the LTS branch

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants