Skip to content

Conversation

daipom
Copy link
Contributor

@daipom daipom commented Aug 18, 2025

Which issue(s) this PR fixes:
Continued from

What this PR does / why we need it:
Each test should not consider the initialization of Fluent::Engine.
It should be the responsibility of Fluent::Test.setup.

Note: Set nil explicitly to ensure that GC can remove the objects, though it is very strange that some objects can still exist after remove_const and GC.start.

Docs Changes:
Not needed.

Release Note:
CI improvements.

Continued from 6cac9f0.

Each test should not consider the initialization of
`Fluent::Engine`.
It should be the responsibility of `Fluent::Test.setup`.

Note: Set `nil` explicitly to ensure that GC can remove the
objects, though it is very strange that some objects can still
exist after `remove_const` and `GC.start`.

Signed-off-by: Daijiro Fukuda <fukuda@clear-code.com>
@daipom daipom added this to the v1.19.1 milestone Aug 18, 2025
@daipom daipom added CI Test/CI issues backport to v1.16 We will backport this fix to the LTS branch labels Aug 18, 2025
@daipom daipom requested review from Watson1978 and kenhys August 18, 2025 04:44
Copy link
Contributor

@Watson1978 Watson1978 left a comment

Choose a reason for hiding this comment

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

👍🏻

@daipom
Copy link
Contributor Author

daipom commented Aug 18, 2025

Thanks for your review!

@daipom daipom merged commit 2796e84 into fluent:master Aug 18, 2025
17 checks passed
@daipom daipom deleted the test-ensure-remove-old-engine-on-setup branch August 18, 2025 06:31
@daipom daipom modified the milestones: v1.19.1, v1.20.0 Aug 21, 2025
daipom added a commit to daipom/fluentd that referenced this pull request Sep 10, 2025
**Which issue(s) this PR fixes**:
Continued from

* fluent#5054
* fluent#5055 (6cac9f0)

**What this PR does / why we need it**:
Each test should not consider the initialization of `Fluent::Engine`.
It should be the responsibility of `Fluent::Test.setup`.

Note: Set `nil` explicitly to ensure that GC can remove the objects,
though it is very strange that some objects can still exist after
`remove_const` and `GC.start`.

**Docs Changes**:
Not needed.

**Release Note**:
CI improvements.

Signed-off-by: Daijiro Fukuda <fukuda@clear-code.com>
daipom added a commit to daipom/fluentd that referenced this pull request Sep 11, 2025
**Which issue(s) this PR fixes**:
Continued from

* fluent#5054
* fluent#5055 (6cac9f0)

**What this PR does / why we need it**:
Each test should not consider the initialization of `Fluent::Engine`.
It should be the responsibility of `Fluent::Test.setup`.

Note: Set `nil` explicitly to ensure that GC can remove the objects,
though it is very strange that some objects can still exist after
`remove_const` and `GC.start`.

**Docs Changes**:
Not needed.

**Release Note**:
CI improvements.

Signed-off-by: Daijiro Fukuda <fukuda@clear-code.com>
daipom added a commit to daipom/fluentd that referenced this pull request Sep 11, 2025
**Which issue(s) this PR fixes**:
Continued from

* fluent#5054
* fluent#5055 (6cac9f0)

**What this PR does / why we need it**:
Each test should not consider the initialization of `Fluent::Engine`.
It should be the responsibility of `Fluent::Test.setup`.

Note: Set `nil` explicitly to ensure that GC can remove the objects,
though it is very strange that some objects can still exist after
`remove_const` and `GC.start`.

**Docs Changes**:
Not needed.

**Release Note**:
CI improvements.

Signed-off-by: Daijiro Fukuda <fukuda@clear-code.com>
daipom added a commit to daipom/fluentd that referenced this pull request Sep 11, 2025
**Which issue(s) this PR fixes**:
Continued from

* fluent#5054
* fluent#5055 (6cac9f0)

**What this PR does / why we need it**:
Each test should not consider the initialization of `Fluent::Engine`.
It should be the responsibility of `Fluent::Test.setup`.

Note: Set `nil` explicitly to ensure that GC can remove the objects,
though it is very strange that some objects can still exist after
`remove_const` and `GC.start`.

**Docs Changes**:
Not needed.

**Release Note**:
CI improvements.

Signed-off-by: Daijiro Fukuda <fukuda@clear-code.com>
daipom added a commit that referenced this pull request Sep 11, 2025
…5085)

**Which issue(s) this PR fixes**:
* Backport #5057

Continued from

* #5054
* #5055 (6cac9f0)

**What this PR does / why we need it**:
Each test should not consider the initialization of `Fluent::Engine`. It
should be the responsibility of `Fluent::Test.setup`.

Note: Set `nil` explicitly to ensure that GC can remove the objects,
though it is very strange that some objects can still exist after
`remove_const` and `GC.start`.

**Docs Changes**:
Not needed.

**Release Note**:
CI improvements.

Signed-off-by: Daijiro Fukuda <fukuda@clear-code.com>
@daipom daipom added the backported "backport to LTS" is done label Sep 11, 2025
@daipom daipom modified the milestones: v1.20.0, v1.19.1 Sep 11, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport to v1.16 We will backport this fix to the LTS branch backported "backport to LTS" is done CI Test/CI issues
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants