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

Fix appveyor ci for round4 #2864

Merged
merged 4 commits into from
Mar 10, 2020
Merged

Fix appveyor ci for round4 #2864

merged 4 commits into from
Mar 10, 2020

Conversation

cosmo0920
Copy link
Contributor

Which issue(s) this PR fixes:

Related to #2854.

  • Fix expected position in posfile
  • Use correct error message on connecting error
  • Sort by tag name instead of timestamps which are same integer value (this causes unstable result)

What this PR does / why we need it:
We should take care of AppVeyor CI result.

Docs Changes:
No needed.

Release Note:

None.

@cosmo0920 cosmo0920 requested a review from ganmacs March 6, 2020 07:30
@cosmo0920 cosmo0920 force-pushed the fix-appveyor-ci-for-round4 branch from c102e55 to 75a631c Compare March 6, 2020 07:33
@cosmo0920
Copy link
Contributor Author

Phew, we got green on AppVeyor! https://ci.appveyor.com/project/fluent/fluentd/builds/31285183

Copy link
Member

@ganmacs ganmacs left a comment

Choose a reason for hiding this comment

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

commit message may have a typo. 75a631c

aortable -> sortable ?

@@ -196,7 +196,11 @@ def build_files(file)
@file.seek(0)
lines = @file.readlines
assert_equal 2, lines.size
assert_equal "valid_path\t000000000000000a\t000000000000000b\n", lines[0]
if Fluent.windows?
assert_equal "valid_path\t0000000000000000a\t000000000000000b\n", lines[0]
Copy link
Member

Choose a reason for hiding this comment

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

[question] Why it happens?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'll dig it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Aha! I got it.
In Windows, it is also needed to use binmode for Tempfile! 👊

@cosmo0920 cosmo0920 force-pushed the fix-appveyor-ci-for-round4 branch from 75a631c to 5328234 Compare March 6, 2020 09:49
Winsock generates the different error message than Linux's.

Signed-off-by: Hiroshi Hatake <[email protected]>
Because these test cases should handle same timestamp.
We should sort with another sortable key such as tag.

Signed-off-by: Hiroshi Hatake <[email protected]>
In Windows, writing 0 contained contents should be written as binmode.
Otherwise, it will cause corrupted contents.

Signed-off-by: Hiroshi Hatake <[email protected]>
@cosmo0920 cosmo0920 force-pushed the fix-appveyor-ci-for-round4 branch from 5328234 to 7560a48 Compare March 9, 2020 10:03
@cosmo0920
Copy link
Contributor Author

I'll merge this PR after finishing CIs. 💪

@cosmo0920 cosmo0920 merged commit 66cad23 into master Mar 10, 2020
@cosmo0920 cosmo0920 deleted the fix-appveyor-ci-for-round4 branch March 10, 2020 01:02
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.

2 participants