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

Remove sourcemap/test and elasticsearch/estest #5854

Merged
merged 1 commit into from
Aug 2, 2021

Conversation

axw
Copy link
Member

@axw axw commented Aug 2, 2021

Motivation/summary

Remove unnecessarily fragmented packages used for testing source maps, moving the code directly into the sourcemap package:

  • elasticsearch/estest was only used inside the sourcemap package's tests; remove this package and refactor tests to use an httptest.Server, exercising the code in much the same way as in a real apm-server
  • sourcemap/test had two purposes: providing a sourcemaps for tests, and providing elasticsearch.Clients that would respond with canned responses; remove sourcemap/test, canned responses are now handled per above, sourcemaps used for testing are moved into the sourcemap_test package, and the beater test that was using sourcemap/test now uses testdata/sourcemap instead

Checklist

- [ ] Update CHANGELOG.asciidoc
- [ ] Documentation has been updated

How to test these changes

Non-functional change.

Related issues

Yak shaving related to cherry-picking go-elasticsearch product check changes into #5838

elasticsearch/estest was only used inside the sourcemap
package's tests. Remove this package and refactor tests
to use an httptest.Server where feasible.

Change the beater test that was using sourcemap/test
for test data to use the sourcemap in testdata/sourcemap
instead, and remove sourcemap/test.
@axw axw requested a review from a team August 2, 2021 09:12
@apmmachine
Copy link
Contributor

💚 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 preview

Expand to view the summary

Build stats

  • Start Time: 2021-08-02T08:44:29.164+0000

  • Duration: 44 min 5 sec

  • Commit: 632a218

Test stats 🧪

Test Results
Failed 0
Passed 5934
Skipped 14
Total 5948

Trends 🧪

Image of Build Times

Image of Tests

Copy link
Contributor

@stuartnelson3 stuartnelson3 left a comment

Choose a reason for hiding this comment

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

💯 awesome!

@axw axw marked this pull request as ready for review August 2, 2021 09:55
@axw axw merged commit 7ccc202 into elastic:master Aug 2, 2021
@axw axw deleted the remove-estest branch August 2, 2021 09:55
mergify bot pushed a commit that referenced this pull request Aug 2, 2021
elasticsearch/estest was only used inside the sourcemap
package's tests. Remove this package and refactor tests
to use an httptest.Server where feasible.

Change the beater test that was using sourcemap/test
for test data to use the sourcemap in testdata/sourcemap
instead, and remove sourcemap/test.

(cherry picked from commit 7ccc202)

# Conflicts:
#	elasticsearch/estest/client.go
axw added a commit that referenced this pull request Aug 3, 2021
elasticsearch/estest was only used inside the sourcemap
package's tests. Remove this package and refactor tests
to use an httptest.Server where feasible.

Change the beater test that was using sourcemap/test
for test data to use the sourcemap in testdata/sourcemap
instead, and remove sourcemap/test.

(cherry picked from commit 7ccc202)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants