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

Migrate more system tests to Go #4469

Merged
merged 6 commits into from
Dec 15, 2020
Merged

Conversation

axw
Copy link
Member

@axw axw commented Dec 1, 2020

Motivation/summary

Migrate more system tests to Go. With the instrumentation tests, I deleted the tests for deprecated instrumentation config, and only test the new libbeat instrumentation config.

Checklist

I have considered changes for:
- [ ] documentation
- [ ] logging (add log lines, choose appropriate log selector, etc.)
- [ ] metrics and monitoring (create issue for Kibana team to add metrics to visualizations, e.g. Kibana#44001)

How to test these changes

cd systemtest && go test -v

Related issues

None.

@apmmachine
Copy link
Contributor

apmmachine commented Dec 1, 2020

💚 Build Succeeded

the below badges are clickable and redirect to their specific view in the CI or DOCS
Pipeline View Test View Changes Artifacts preview

Expand to view the summary

Build stats

  • Build Cause: Pull request #4469 updated

  • Start Time: 2020-12-15T05:14:53.766+0000

  • Duration: 40 min 29 sec

Test stats 🧪

Test Results
Failed 0
Passed 4613
Skipped 124
Total 4737

Steps errors 3

Expand to view the steps failures

Compress
  • Took 0 min 0 sec . View more details on here
  • Description: tar --exclude=coverage-files.tgz -czf coverage-files.tgz coverage
Compress
  • Took 0 min 0 sec . View more details on here
  • Description: tar --exclude=system-tests-linux-files.tgz -czf system-tests-linux-files.tgz system-tests
Test Sync
  • Took 3 min 12 sec . View more details on here
  • Description: ./.ci/scripts/sync.sh

@axw axw requested a review from a team December 1, 2020 09:53
Copy link
Contributor

@simitt simitt left a comment

Choose a reason for hiding this comment

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

Removing the extra tests for apm-server.instrumentation is fine.
Just a nit - please remove the libbeat_instrumentation_enabled parts from the tests/system/config/apm-server.yml.j2

@codecov-io
Copy link

Codecov Report

Merging #4469 (88f341b) into master (750ea89) will increase coverage by 0.02%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##           master    #4469      +/-   ##
==========================================
+ Coverage   75.95%   75.97%   +0.02%     
==========================================
  Files         161      161              
  Lines        9787     9787              
==========================================
+ Hits         7434     7436       +2     
+ Misses       2353     2351       -2     
Impacted Files Coverage Δ
...ack/apm-server/aggregation/txmetrics/aggregator.go 93.36% <0.00%> (ø)
publish/acker.go 89.47% <0.00%> (+10.52%) ⬆️

@axw
Copy link
Member Author

axw commented Dec 15, 2020

Just a nit - please remove the libbeat_instrumentation_enabled parts from the tests/system/config/apm-server.yml.j2

Done, thanks for the nudge

@axw axw merged commit 1fd28d8 into elastic:master Dec 15, 2020
@axw axw deleted the systemtest-migrate-profiling branch December 15, 2020 05:56
simitt pushed a commit to simitt/apm-server that referenced this pull request Dec 15, 2020
* systemtest: migrate profiling tests to Go

* systemtest: migrate remaining instrumentation

* systemtest: migrate another sampling-related test

* systemtest: migrate some API Key command tests

* tests/system/config: drop instrumentation bits
simitt added a commit that referenced this pull request Dec 15, 2020
* systemtest: migrate profiling tests to Go

* systemtest: migrate remaining instrumentation

* systemtest: migrate another sampling-related test

* systemtest: migrate some API Key command tests

* tests/system/config: drop instrumentation bits

Co-authored-by: Andrew Wilkins <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants