Fixing flaky tests for windows 7 32bit by narph · Pull Request #23480 · elastic/beats

narph · 2021-01-13T09:49:53Z

Will update soon

What does this PR do?

After investigation we see 2 main issues:

constant .... overflows int

Because the int value is overflowed, the fix is to specify the int64 type instead.

Tests failing consistently most likely due to expfmt package. This happens in function https://github.com/elastic/beats/blob/master/metricbeat/helper/prometheus/prometheus.go#L72
When trying to read and parse the data, lines for example:

etcd_disk_wal_fsync_duration_seconds_bucket{le="0.002"} 0	
etcd_disk_wal_fsync_duration_seconds_bucket{le="0.004"} 0

are not considered.

Why is it important?

Should fix/skip the following:
(etcd failing tests were previously fixed)

Python tests

Checklist

My code follows the style guidelines of this project
I have commented my code, particularly in hard-to-understand areas
I have made corresponding changes to the documentation
I have made corresponding change to the default configuration files
I have added tests that prove my fix is effective or that my feature works
I have added an entry in CHANGELOG.next.asciidoc or CHANGELOG-developer.next.asciidoc.

Related issues

Closes [Metricbeat] in windows-7-32bits got some flakiness tests #19835

elasticmachine · 2021-01-13T10:01:36Z

💚 Build Succeeded

the below badges are clickable and redirect to their specific view in the CI or DOCS

Expand to view the summary

Build stats

Build Cause: narph commented: /test
- Start Time: 2021-01-18T09:26:51.675+0000
Duration: 48 min 39 sec
Commit: 871f370

Test stats 🧪

Test	Results
Failed	0
Passed	10070
Skipped	2468
Total	12538

💚 Flaky test report

Tests succeeded.

Expand to view the summary

Test stats 🧪

Test	Results
Failed	0
Passed	10070
Skipped	2468
Total	12538

narph · 2021-01-13T12:10:44Z

@v1v , most of tests are failing consistently most likely due to expfmt package. This happens in function https://github.com/elastic/beats/blob/master/metricbeat/helper/prometheus/prometheus.go#L72
When trying to read and parse the data, lines for example:

etcd_disk_wal_fsync_duration_seconds_bucket{le="0.002"} 0	
etcd_disk_wal_fsync_duration_seconds_bucket{le="0.004"} 0

are not considered.

If we change the value 0 to 1 then everything is fine. This PR fixes the etcd module tests. I have upgraded the package but error still consists. I am not sure there is a solution for this tests for 32 bit os's.
What should we do in this case? Update test files? Make sure we skip these specific tests for this os?

windows stage is mandatory, but optional stages are not mandatory for PRs

elasticmachine · 2021-01-13T21:37:53Z

Pinging @elastic/integrations (Team:Integrations)

narph · 2021-01-14T10:37:41Z

running the tests again

/test

narph · 2021-01-14T10:43:31Z

/test

metricbeat/module/ceph/cluster_status/cluster_status_test.go

metricbeat/module/etcd/metrics/_meta/testdata/metrics.plain

jsoriano · 2021-01-14T11:14:35Z

metricbeat/module/etcd/metrics/metrics_test.go

 // under the License.

-// +build !integration
+// +build !integration, !386


Are these tests failing in all 32-bit platforms or only in Windows?

only windows 32 bit

Could we explicitly skip the tests only for Windows 32 bits instead of for all 386 platforms? With a comment to know what they are skipped.

sure, I have made the changes

jsoriano · 2021-01-14T11:16:13Z

metricbeat/module/system/test_system.py


            assert isinstance(udp["all"]["count"], int)

+    @unittest.skipIf(sys.platform == "win32", "Flaky test")


Flaky tests should have a open issue to address them in the future.

I would have created one separately but I had around 40+ tests, it would have been hard to manage the tickets, I have taken screenshots with all the ones that kept on popping up

Umm, does this test always fail in win32? If it always fail maybe instead of skipping it because of flakiness we can say in the comment that this test (or this feature) is not supported in win 32.

the test does not always fail, it passed previously.

If we plan to fix these flakiness at some moment we should have an issue for that, and reference it in the comment. Flaky tests can be hiding bugs.
But possibly it doesn't make much sense to dedicate a lot of effort to corner cases in 32-bits Windows 🙂

v1v · 2021-01-14T12:36:03Z

I've just added all the labels for all the windows that we support to validate there are no any regressions.

/test

narph · 2021-01-14T12:42:44Z

/test

jsoriano

Thanks for working on this!

jsoriano · 2021-01-14T13:56:26Z

metricbeat/module/system/test_system.py


            assert isinstance(udp["all"]["count"], int)

+    @unittest.skipIf(sys.platform == "win32", "Flaky test")


If we plan to fix these flakiness at some moment we should have an issue for that, and reference it in the comment. Flaky tests can be hiding bugs.
But possibly it doesn't make much sense to dedicate a lot of effort to corner cases in 32-bits Windows 🙂

narph · 2021-01-14T16:20:00Z

/test

narph · 2021-01-18T09:26:22Z

/test

* enabling testing on windows 32 bit * update test file * fix * [CI] Optional stage instead the mandatory one windows stage is mandatory, but optional stages are not mandatory for PRs * fix int issue * ignore 386 architecture * skip tests * skip test * function redeclared * re write * add windows constraint * add tag * add comments Co-authored-by: Victor Martinez <VictorMartinezRubio@gmail.com> (cherry picked from commit 1b43637)

enabling testing on windows 32 bit

4bbc46a

botelastic bot added the needs_team Indicates that the issue/PR needs a Team:* label label Jan 13, 2021

narph self-assigned this Jan 13, 2021

update test file

09279b0

fix

c88333a

narph changed the title ~~Fixing flaky tests for windows 7 32bit~~ Fixing flakey tests for windows 7 32bit Jan 13, 2021

v1v added the windows-7-32 Enable builds in the CI for windows-7 32 bits label Jan 13, 2021

v1v and others added 3 commits January 13, 2021 13:03

[CI] Optional stage instead the mandatory one

c3f893b

windows stage is mandatory, but optional stages are not mandatory for PRs

fix int issue

739b814

ignore 386 architecture

a9175ec

narph changed the title ~~Fixing flakey tests for windows 7 32bit~~ Fixing flaky tests for windows 7 32bit Jan 13, 2021

narph added 2 commits January 13, 2021 16:43

skip tests

aa3f379

skip test

2c454d9

andresrc added the Team:Integrations Label for the Integrations team label Jan 13, 2021

botelastic bot removed the needs_team Indicates that the issue/PR needs a Team:* label label Jan 13, 2021

function redeclared

cbe6078

Merge branch 'master' into flaky-32bit

50d0aec

jsoriano reviewed Jan 14, 2021

View reviewed changes

re write

ba57b1d

narph requested a review from jsoriano January 14, 2021 11:33

v1v added windows-10 Enable builds in the CI for windows-10 windows-2008 Enable builds in the CI for windows-2008 windows-2012 Enable builds in the CI for windows-2012 windows-2016 Enable builds in the CI for windows-2016 windows-7 Enable builds in the CI for windows-7 labels Jan 14, 2021

v1v added the windows-8 Enable builds in the CI for windows-8 label Jan 14, 2021

narph added 3 commits January 14, 2021 14:13

add windows constraint

efb4d27

add tag

914924d

add comments

871f370

jsoriano approved these changes Jan 14, 2021

View reviewed changes

narph merged commit 1b43637 into elastic:master Jan 19, 2021

narph deleted the flaky-32bit branch January 19, 2021 09:07

narph mentioned this pull request Jan 19, 2021

Cherry-pick #23480 to 7.x: Fixing flaky tests for windows 7 32bit #23558

Merged

6 tasks

narph added the v7.12.0 label Jan 19, 2021

narph mentioned this pull request Jan 19, 2021

Cherry-pick #23480 to 7.11: Fixing flaky tests for windows 7 32bit #23559

Merged

6 tasks

narph added the v7.11.0 label Jan 19, 2021


		assert isinstance(udp["all"]["count"], int)

		@unittest.skipIf(sys.platform == "win32", "Flaky test")

Conversation

narph commented Jan 13, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What does this PR do?

Why is it important?

Checklist

Related issues

Uh oh!

elasticmachine commented Jan 13, 2021 • edited by jenkins-beats-ci bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

💚 Build Succeeded

Build stats

Test stats 🧪

💚 Flaky test report

Test stats 🧪

Uh oh!

narph commented Jan 13, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

elasticmachine commented Jan 13, 2021

Uh oh!

narph commented Jan 14, 2021

Uh oh!

narph commented Jan 14, 2021

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

v1v commented Jan 14, 2021

Uh oh!

narph commented Jan 14, 2021

Uh oh!

jsoriano left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

narph commented Jan 14, 2021

Uh oh!

narph commented Jan 18, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

narph commented Jan 13, 2021 •

edited

Loading

elasticmachine commented Jan 13, 2021 •

edited by jenkins-beats-ci bot

Loading

narph commented Jan 13, 2021 •

edited

Loading