Run confgenerator tests cross platform, where possible #72

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

igorpeshansky merged 16 commits into GoogleCloudPlatform:master from observIQ:cross-platform-configgen-tests

Jun 8, 2021

Contributor

djaglowski commented May 5, 2021 •

edited

Loading

This PR allows confgenerator tests to run cross-platform. The tests also now run in parallel.

Contributor Author

djaglowski commented May 5, 2021

//cc @quentinmit

Contributor

qingling128 commented May 7, 2021

The latest commit https://github.com/GoogleCloudPlatform/ops-agent/runs/2514781382 from the master branch seems to be pass the unit tests for Windows: https://travis-ci.com/github/GoogleCloudPlatform/ops-agent/jobs/503300262. Perhaps quentin-otlinux needs another rebase.

BTW, we are running into the same issue that this PR is trying to solve. Some related conversation about running unit tests cross platforms: #69 (comment) in order to easily run both Linux side and Windows side of the config generation tests from a Linux workstation. Currently the Windows side of the config generation tests need to be run from a Windows VM.

quentinmit force-pushed the quentin-otlinux branch from 2aa1430 to 3161c2e Compare

May 11, 2021 23:06

quentinmit force-pushed the quentin-otlinux branch from 56d9ddb to 1dccdc5 Compare

June 1, 2021 19:06

Contributor

qingling128 commented Jun 2, 2021

FYI that #85 (aka quentin-otlinux) has been merged to master now. To get this PR ready for review, could you rebase off the latest master?

djaglowski changed the base branch from quentin-otlinux to master

June 3, 2021 18:20

djaglowski commented

View reviewed changes

confgenerator/testdata/valid/windows/default_config/golden_otel.conf Outdated Show resolved Hide resolved

djaglowski added 2 commits

June 3, 2021 15:27


          Allow confgen tests to run cross platform (exluding FluentBit)

cb5d871


          Fix default fluent bit config on windows test

51bc739

djaglowski commented

View reviewed changes

confgenerator/testdata/valid/windows/default_config/golden_fluent_bit_main.conf Show resolved Hide resolved

djaglowski marked this pull request as ready for review

June 3, 2021 20:33

Contributor Author

djaglowski commented Jun 3, 2021

@qingling128 I've rebased onto master and believe this is ready for review.

igorpeshansky suggested changes

View reviewed changes

.gitignore Outdated Show resolved Hide resolved

confgenerator/confgenerator_test.go Outdated Show resolved Hide resolved

confgenerator/confgenerator_test.go Outdated Show resolved Hide resolved

confgenerator/confgenerator_test.go Outdated Show resolved Hide resolved

confgenerator/confgenerator_test.go Outdated Show resolved Hide resolved

confgenerator/confgenerator_test.go Outdated Show resolved Hide resolved

confgenerator/confgenerator_test.go Outdated Show resolved Hide resolved

confgenerator/testdata/valid/windows/default_config/golden_fluent_bit_main.conf Show resolved Hide resolved

confgenerator/testdata/valid/windows/default_config/golden_otel.conf Outdated Show resolved Hide resolved

confgenerator/confgenerator_test.go Outdated Show resolved Hide resolved

djaglowski added 4 commits

June 7, 2021 10:49


          Address PR feedback

8d094b7


          goimports

d447edd


          Also run fluentbit config generation tests cross platform.

b7f66c8


          Undo combination of helper and top-level test funcs

a6c546e

igorpeshansky suggested changes

View reviewed changes

confgenerator/confgenerator_test.go Outdated Show resolved Hide resolved

confgenerator/confgenerator_test.go Outdated Show resolved Hide resolved

confgenerator/confgenerator_test.go Outdated Show resolved Hide resolved

confgenerator/confgenerator_test.go Outdated Show resolved Hide resolved

confgenerator/confgenerator_test.go Outdated Show resolved Hide resolved

igorpeshansky reviewed

View reviewed changes

Contributor

igorpeshansky left a comment

Sorry, one more thing…

confgenerator/confgenerator_test.go Show resolved Hide resolved


          Rework filepathJoin. Tighten up usage of t.Errorf vs t.Fatalf

0e10421

Contributor Author

djaglowski commented Jun 7, 2021

@igorpeshansky, I appreciate the feedback. I've learned a few nuances to golang.

djaglowski added 5 commits

June 7, 2021 14:35


          Parallelize confgenerator tests

2fc98d4


          Make platformConfig fully pass-by-value

13fd216


          Actually fix parallelism in confgenerator

9c1118f


          Remove coverage report file

fdd6a07


          Add validation of filepathJoin override

326fcef

igorpeshansky suggested changes

View reviewed changes

confgenerator/confgenerator_test.go Show resolved Hide resolved

confgenerator/confgenerator_test.go Outdated Show resolved Hide resolved

confgenerator/confgenerator_test.go Outdated Show resolved Hide resolved

confgenerator/confgenerator_test.go Outdated Show resolved Hide resolved

confgenerator/confgenerator_test.go Show resolved Hide resolved


          Use custom filepathJoin in confgenerator. Standardize signatures.

b24e166

igorpeshansky suggested changes

View reviewed changes

confgenerator/confgenerator_test.go Show resolved Hide resolved

confgenerator/confgenerator_test.go Outdated Show resolved Hide resolved

confgenerator/confgenerator_test.go Show resolved Hide resolved


          Use real filepath.Join in production executable, but override for tests

9139d71


          merge master

5c7290e

igorpeshansky suggested changes

View reviewed changes

Contributor

igorpeshansky left a comment

Thanks for bearing with me. Some minor remaining tweaks.

confgenerator/confgenerator_test.go Show resolved Hide resolved

confgenerator/confgenerator_test.go Outdated Show resolved Hide resolved

confgenerator/confgenerator_test.go Outdated Show resolved Hide resolved


          Incorporate PR feedback

c10f43e

igorpeshansky approved these changes

View reviewed changes

Contributor

igorpeshansky left a comment

LGTM

Thank you for the contribution!

igorpeshansky merged commit 69cebd0 into GoogleCloudPlatform:master

djaglowski deleted the cross-platform-configgen-tests branch

June 9, 2021 00:06

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet