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

end2end tests part 1 #316

Open
wants to merge 9 commits into
base: main
Choose a base branch
from
Open

end2end tests part 1 #316

wants to merge 9 commits into from

Conversation

trzysiek
Copy link
Member

@trzysiek trzysiek commented Jun 11, 2024

MVP of e2e tests.
I'll write some README, but maybe in next PR, once this is accepted.

You can e.g. see below, that it's often the case that count responses from Quesma and Elastic differ by 1 for the basic Explorer's date_histogram, which is the first testcase I "recorded" and included. It's most likely because of a timezone bug that I mentioned for a second during today's standup (#307)
(error and info messages on both screens would already be better)
Screenshot 2024-06-12 at 00 52 45

Screenshot 2024-06-12 at 13 02 44

  • When running Quesma locally, all interesting (so far _(async_)search and _field_caps) requests are being saved.
  • You can run script bin/save_test.py which will copy either all of those requests, or some subset chosen by you, to a proper directory with testcases: quesma/tests/end2end/testcases/{new-testcase-nr}/
    I've done it and saved first 2 requests.
  • When you have Quesma with local-dev-dual-comparison.yml config running, you can simply run go test and all requests from all testcases will be run via Elastic/Quesma+Clickhouse, and responses compared.

Of course there's lots of TODO, but it's something, and it seems to work.

Happy to hear any feedback/suggestions.

@trzysiek trzysiek force-pushed the e2e-1 branch 13 times, most recently from f8119e8 to 4ce7419 Compare June 12, 2024 14:25
@trzysiek trzysiek marked this pull request as ready for review June 12, 2024 14:25
@trzysiek trzysiek requested a review from a team as a code owner June 12, 2024 14:25
@trzysiek trzysiek force-pushed the e2e-1 branch 2 times, most recently from 4400d63 to 8931633 Compare June 12, 2024 15:19
Copy link
Member

@nablaone nablaone left a comment

Choose a reason for hiding this comment

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

Overall it's the best attempt so far.

mitmproxy addon need some changes.

docker/local-dev.yml Outdated Show resolved Hide resolved
bin/save_test.py Outdated
@@ -0,0 +1,79 @@
#!/usr/bin/env python3
Copy link
Member

Choose a reason for hiding this comment

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

Maybe we can use more specific name here: e2e_save_tests.py

Copy link
Member

Choose a reason for hiding this comment

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

We should be able to control recording. Right now all request are being recorded.

Example recording scenario:

./bin/e2e_start_recording.py

# some kibana exploration here

./bin/e2e_stop_recording.py

./bin/e2e_save_tests.py
# or
./bin/e2e_clean_recording.py

Copy link
Contributor

Choose a reason for hiding this comment

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

I would even add own directory for that:
./bin/e2e/start_recording.py

self.w.add(flow)


writer = Writer()
Copy link
Member

Choose a reason for hiding this comment

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

mitmproxy doesn't start on my setup:

Screenshot 2024-06-14 at 13 25 46

@@ -0,0 +1,142 @@
package end2end
Copy link
Member

Choose a reason for hiding this comment

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

File should be named e2e_integration_test.go and has following build tag

//go:build integration

otherwise tests will be run as normal unit tests.

Copy link
Contributor

Choose a reason for hiding this comment

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

I would also keep e2e name consistent if possible

Copy link
Contributor

@jakozaur jakozaur left a comment

Choose a reason for hiding this comment

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

The idea is great, but doesn't work. Moreover it breaks other use cases.

bin/save_test.py Outdated
@@ -0,0 +1,79 @@
#!/usr/bin/env python3
Copy link
Contributor

Choose a reason for hiding this comment

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

I would even add own directory for that:
./bin/e2e/start_recording.py

@@ -0,0 +1,142 @@
package end2end
Copy link
Contributor

Choose a reason for hiding this comment

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

I would also keep e2e name consistent if possible

@jakozaur
Copy link
Contributor

Error I see:

2024-06-14 15:41:28 [13:41:28.087] error in script /var/mitmproxy/test_saver.py
2024-06-14 15:41:28 Traceback (most recent call last):
2024-06-14 15:41:28   File "/var/mitmproxy/test_saver.py", line 26, in <module>
2024-06-14 15:41:28     writer = Writer()
2024-06-14 15:41:28              ^^^^^^^^
2024-06-14 15:41:28   File "/var/mitmproxy/test_saver.py", line 15, in __init__
2024-06-14 15:41:28     self.w = io.FlowWriter()
2024-06-14 15:41:28              ^^^^^^^^^^^^^^^
2024-06-14 15:41:28 TypeError: FlowWriter.__init__() missing 1 required positional argument: 'fo'
2024-06-14 15:41:28 Error logged during startup, exiting...
2024-06-14 15:41:28 usermod: no changes

@jakozaur
Copy link
Contributor

Ok, I started to understand how this works:
#334

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