Skip to content

[chore] Fix TestPerfCounter_Reset test #36726

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.

Sign up for GitHub

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

Jump to bottom
Merged
dmitryax merged 2 commits into from
Dec 9, 2024
Merged

[chore] Fix TestPerfCounter_Reset test #36726

dmitryax merged 2 commits into from
Dec 9, 2024

Conversation

@pjanotti
Copy link
Contributor

@pjanotti pjanotti commented Dec 9, 2024 •
edited
Loading

Description

Fix the TestPerfCounter_Reset that is failing consistently.

=== FAIL: . TestPerfCounter_Reset (0.00s)
    watcher_test.go:131: 
        	Error Trace:	D:/a/opentelemetry-collector-contrib/opentelemetry-collector-contrib/pkg/winperfcounters/watcher_test.go:131
        	Error:      	Both arguments must be pointers
        	Test:       	TestPerfCounter_Reset

Root case: after upgrading testify circa of 2 weeks ago, since it included:

NotSame should fail if args are not pointers stretchr/testify#1661 by @sikehish in stretchr/testify#1664

See https://github.com/stretchr/testify/releases

Testing

Local test run

@pjanotti pjanotti requested review from a team and dashpole as code owners December 9, 2024 16:36
@pjanotti pjanotti added the Run Windows Enable running windows test on a PR label Dec 9, 2024
@github-actions github-actions bot requested review from Mrod1598 and alxbl December 9, 2024 16:37
codeboten
codeboten approved these changes Dec 9, 2024
View reviewed changes
Copy link
Contributor

@codeboten codeboten left a comment

Choose a reason for hiding this comment

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

Ugh :( this one again... i had to fix a bunch of these because of the change in testify

@pjanotti
Copy link
Contributor Author

pjanotti commented Dec 9, 2024

@codeboten I would love to have Run Windows on (or always running Windows without the label) to avoid such "Windows" cases - tracked via #34385

@pjanotti
Copy link
Contributor Author

pjanotti commented Dec 9, 2024

CI failures:

Windows failures are #36481

OPAMP failure: https://github.com/open-telemetry/opentelemetry-collector-contrib/actions/runs/12242789600/job/34151180181?pr=36726#step:8:823

githubgen on check-codeowners: https://github.com/open-telemetry/opentelemetry-collector-contrib/actions/runs/12242789468/job/34150903475?pr=36726#step:4:25

@dmitryax dmitryax merged commit 5ab175c into open-telemetry:main Dec 9, 2024
172 of 175 checks passed
@dmitryax
Copy link
Member

dmitryax commented Dec 9, 2024

Thanks @pjanotti!

@github-actions github-actions bot added this to the next release milestone Dec 9, 2024
@pjanotti pjanotti deleted the fix-pkg-winperfcountes-test branch December 9, 2024 20:28
@songy23 songy23 mentioned this pull request Dec 9, 2024
Closed
sbylica-splunk pushed a commit to sbylica-splunk/opentelemetry-collector-contrib that referenced this pull request Dec 17, 2024
@pjanotti @atoulme @sbylica-splunk
[chore] Fix TestPerfCounter_Reset test (open-telemetry#36726)
24c6a36 
#### Description

Fix the TestPerfCounter_Reset that is failing consistently. 

```terminal
=== FAIL: . TestPerfCounter_Reset (0.00s)
    watcher_test.go:131: 
        	Error Trace:	D:/a/opentelemetry-collector-contrib/opentelemetry-collector-contrib/pkg/winperfcounters/watcher_test.go:131
        	Error:      	Both arguments must be pointers
        	Test:       	TestPerfCounter_Reset
```

Root case: after upgrading `testify` circa of 2 weeks ago, since it
included:

> NotSame should fail if args are not pointers
stretchr/testify#1661 by @sikehish in
stretchr/testify#1664

See https://github.com/stretchr/testify/releases

#### Testing

Local test run

Co-authored-by: Antoine Toulme <[email protected]>
AkhigbeEromo pushed a commit to sematext/opentelemetry-collector-contrib that referenced this pull request Jan 13, 2025
@pjanotti @atoulme @AkhigbeEromo
[chore] Fix TestPerfCounter_Reset test (open-telemetry#36726)
a723578 
#### Description

Fix the TestPerfCounter_Reset that is failing consistently. 

```terminal
=== FAIL: . TestPerfCounter_Reset (0.00s)
    watcher_test.go:131: 
        	Error Trace:	D:/a/opentelemetry-collector-contrib/opentelemetry-collector-contrib/pkg/winperfcounters/watcher_test.go:131
        	Error:      	Both arguments must be pointers
        	Test:       	TestPerfCounter_Reset
```

Root case: after upgrading `testify` circa of 2 weeks ago, since it
included:

> NotSame should fail if args are not pointers
stretchr/testify#1661 by @sikehish in
stretchr/testify#1664

See https://github.com/stretchr/testify/releases

#### Testing

Local test run

Co-authored-by: Antoine Toulme <[email protected]>
zeck-ops pushed a commit to zeck-ops/opentelemetry-collector-contrib that referenced this pull request Apr 23, 2025
@pjanotti @atoulme @zeck-ops
[chore] Fix TestPerfCounter_Reset test (open-telemetry#36726)
95ef54c 
#### Description

Fix the TestPerfCounter_Reset that is failing consistently. 

```terminal
=== FAIL: . TestPerfCounter_Reset (0.00s)
    watcher_test.go:131: 
        	Error Trace:	D:/a/opentelemetry-collector-contrib/opentelemetry-collector-contrib/pkg/winperfcounters/watcher_test.go:131
        	Error:      	Both arguments must be pointers
        	Test:       	TestPerfCounter_Reset
```

Root case: after upgrading `testify` circa of 2 weeks ago, since it
included:

> NotSame should fail if args are not pointers
stretchr/testify#1661 by @sikehish in
stretchr/testify#1664

See https://github.com/stretchr/testify/releases

#### Testing

Local test run

Co-authored-by: Antoine Toulme <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@atoulme atoulme atoulme approved these changes

@codeboten codeboten codeboten approved these changes

@dmitryax dmitryax dmitryax approved these changes

@dashpole dashpole Awaiting requested review from dashpole dashpole is a code owner

@alxbl alxbl Awaiting requested review from alxbl

@Mrod1598 Mrod1598 Awaiting requested review from Mrod1598

Assignees

@fatsheep9146 fatsheep9146

Labels

os:windows pkg/winperfcounters Run Windows Enable running windows test on a PR

Projects

None yet

Milestone

v0.116.0

Development

Successfully merging this pull request may close these issues.

5 participants

@pjanotti @dmitryax @atoulme @codeboten @fatsheep9146