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

test_in_tail: Fix unstable tests #3767

Merged
merged 8 commits into from Jun 2, 2022
Merged

test_in_tail: Fix unstable tests #3767

merged 8 commits into from Jun 2, 2022

Conversation

ashie
Copy link
Member

@ashie ashie commented May 31, 2022

Which issue(s) this PR fixes:
None

What this PR does / why we need it:

  • 3fbcc11: The rotation tests are wrong on Windows. sub_test_rotate_file should pass a rotated file to the block, not a new file.
    In addition, it should be opened by Fluent::FileWrapper to avoid locking.
  • 90ad37d, bcba06f: Create a new empty directory with a random path on each tests to avoid locking files by Windows OS.
  • a065d02: Simplify deleting temp directory. The previous hacky way is no longer needed since we always create a new directory for now.
    In addition, the previous implementations has some bugs, for example FileUtils.rm_f doesn't have secure option even though the latest Ruby (v3.1), probably it intend FileUtils.rm_r.
  • 4f3b7f3: Drop CI for Ruby 2.6
  • e041b61: Fix some omitted tests on Windows

Docs Changes:
None

Release Note:
Same with the title.

@ashie
Copy link
Member Author

ashie commented May 31, 2022

CI fails on all windows versions.
Still something might wrong, I'll check it.

@ashie ashie marked this pull request as draft May 31, 2022 07:41
@ashie ashie force-pushed the fix-wrong-rotation-tests branch 2 times, most recently from 416160d to f6994e9 Compare May 31, 2022 09:33
ashie added 2 commits June 1, 2022 10:54
`sub_test_rotate_file` should pass a rotated file to block, not a new
file. In addition, it should be opened by `Fluent::FileWrapper` to avoid
locking.

Signed-off-by: Takuro Ashie <ashie@clear-code.com>
…qual`

Signed-off-by: Takuro Ashie <ashie@clear-code.com>
@ashie ashie force-pushed the fix-wrong-rotation-tests branch from 4e78a31 to d5b74b1 Compare June 1, 2022 02:44
Signed-off-by: Takuro Ashie <ashie@clear-code.com>
@ashie ashie force-pushed the fix-wrong-rotation-tests branch from b8f0261 to 65af256 Compare June 1, 2022 04:54
@ashie
Copy link
Member Author

ashie commented Jun 1, 2022

Hmm, it seems that unstable tests are caused by failure of removing previous test files:

2022-06-01T05:30:00.7098380Z     test_emit[flat]:					Failed to clean up D:/a/fluentd/fluentd/test/plugin/../tmp/tail/tail.pos: Permission denied @ rb_file_s_rename - (D:/a/fluentd/fluentd/test/plugin/../tmp/tail/tail.pos, D:/a/_temp/b5cdf7a1924ec6b011fd)
2022-06-01T05:30:01.2203798Z F
2022-06-01T05:30:01.2204492Z ===============================================================================
2022-06-01T05:30:01.2205097Z Failure: test_emit[flat](TailInputTest::singleline)
2022-06-01T05:30:01.2223307Z D:/a/fluentd/fluentd/test/plugin/test_in_tail.rb:474:in `test_emit'
2022-06-01T05:30:01.2223863Z      471: 
2022-06-01T05:30:01.2224264Z      472:       events = d.events
2022-06-01T05:30:01.2224725Z      473:       assert_equal(true, events.length > 0)
2022-06-01T05:30:01.2225182Z   => 474:       assert_equal({"message" => "test3"}, events[0][2])
2022-06-01T05:30:01.2225718Z      475:       assert_equal({"message" => "test4"}, events[1][2])
2022-06-01T05:30:01.2226162Z      476:       assert(events[0][1].is_a?(Fluent::EventTime))
2022-06-01T05:30:01.2226613Z      477:       assert(events[1][1].is_a?(Fluent::EventTime))
2022-06-01T05:30:01.2227027Z <{"message"=>"test3"}> expected but was
2022-06-01T05:30:01.2227379Z <{"message"=>"test1"}>
2022-06-01T05:30:01.2230449Z 
2022-06-01T05:30:01.2230792Z diff:
2022-06-01T05:30:01.2233901Z ? {"message"=>"test3"}
2022-06-01T05:30:01.2234430Z ?                  1  
2022-06-01T05:30:01.2235177Z ?                  ?  
2022-06-01T05:30:01.2235548Z ===============================================================================
2022-06-01T05:30:01.2249202Z Failed to clean up D:/a/fluentd/fluentd/test/plugin/../tmp/tail/tail.pos: Permission denied @ rb_file_s_rename - (D:/a/fluentd/fluentd/test/plugin/../tmp/tail/tail.pos, D:/a/_temp/1953f658c0b72881c77d)
2022-06-01T05:30:01.2260335Z : (0.518041)

@ashie ashie force-pushed the fix-wrong-rotation-tests branch from 9dfc0d3 to 944b16d Compare June 1, 2022 07:22
@ashie ashie changed the title test_in_tail: Fix wrong rotation tests on Windows test_in_tail: Fix unstable tests Jun 1, 2022
@ashie ashie marked this pull request as ready for review June 1, 2022 08:29
@ashie ashie force-pushed the fix-wrong-rotation-tests branch from 7224d90 to d8336e5 Compare June 1, 2022 08:53
ashie added 4 commits June 1, 2022 18:33
On windows, sometimes fails to remove old test files, it makes tests
unstable.

Signed-off-by: Takuro Ashie <ashie@clear-code.com>
Since we always create a new test ditectory on each tests, ensuring to
remove old directory in hacky way is no longer needed.
In addition, we already dropped Ruby 2.6 support, and the previous
implementation has some bugs. For example `FileUtils.rm_f` doesn't
have `secure` option even though the latest Ruby (v3.1), probably it
intend `FileUtils.rm_r`.

Signed-off-by: Takuro Ashie <ashie@clear-code.com>
* TestWithSystem should call super at setup and teadown
* EX_* should be placed just under TailInputTest

Signed-off-by: Takuro Ashie <ashie@clear-code.com>
Signed-off-by: Takuro Ashie <ashie@clear-code.com>
@ashie ashie force-pushed the fix-wrong-rotation-tests branch from d8336e5 to 4f3b7f3 Compare June 1, 2022 09:39
@ashie ashie requested a review from cosmo0920 June 1, 2022 09:42
@ashie
Copy link
Member Author

ashie commented Jun 1, 2022

All tests succeeded by just one try after a long absence 🎉

Signed-off-by: Takuro Ashie <ashie@clear-code.com>
Copy link
Contributor

@cosmo0920 cosmo0920 left a comment

Choose a reason for hiding this comment

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

Looks good for fixing unstable tests on Windows.
Still failing macOS CI tasks but these are not related this fix.

@ashie ashie merged commit f81d60f into master Jun 2, 2022
@ashie ashie deleted the fix-wrong-rotation-tests branch June 2, 2022 02:20
@ashie
Copy link
Member Author

ashie commented Jun 2, 2022

Thanks!

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

2 participants