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

qatool: reduce churn when rerunning #2677

Closed
bassosimone opened this issue Feb 9, 2024 · 0 comments · Fixed by ooni/probe-cli#1515
Closed

qatool: reduce churn when rerunning #2677

bassosimone opened this issue Feb 9, 2024 · 0 comments · Fixed by ooni/probe-cli#1515
Assignees
Labels
bug Something isn't working funder/otffoss2023-2024 techdebt This issue describes technical debt

Comments

@bassosimone
Copy link
Contributor

bassosimone commented Feb 9, 2024

Every time we run qatool there's lots of churn caused by ordering in the minipipeline test suite, which makes it difficult to understand if something actually changed. Maybe we can do something about this?

@bassosimone bassosimone self-assigned this Feb 9, 2024
@bassosimone bassosimone added bug Something isn't working funder/otffoss2023-2024 labels Feb 9, 2024
bassosimone added a commit to ooni/probe-cli that referenced this issue Feb 12, 2024
We add dns.nextdns.io to the QA suite, such that we don't get a
wrong NXDOMAIN when trying to use it as the resolver.

We optionally sort the test keys to ensure there's less churn in
diffs produced when regnerating minipipeline test data.

We fix ./script/updateminipipeline.bash to use ./script/go.bash
for building as opposed to using go.

Closes ooni/probe#2677
@bassosimone bassosimone changed the title LTE: reduce churn when rerunning minipipeline: reduce churn when rerunning Feb 12, 2024
@bassosimone bassosimone changed the title minipipeline: reduce churn when rerunning qatool: reduce churn when rerunning Feb 12, 2024
bassosimone added a commit to ooni/probe-cli that referenced this issue Feb 12, 2024
We add dns.nextdns.io to the QA suite, such that we don't get a wrong
NXDOMAIN when trying to use it as the resolver.

We optionally sort the test keys to ensure there's less churn in diffs
produced when regenerating minipipeline test data.

We fix ./script/updateminipipeline.bash to use ./script/go.bash for
building as opposed to using go.

Also, include endpoint information in `tls_handshake_{start,stop}` and
`http_transaction_{start,stop}` events such that they are sorted
correctly when we sort events to reduce churn. This is necessary because
we're using the endpoint information to sort the network events.

Part of ooni/probe#2677; the churn is still
too high, and I need more changes.
bassosimone added a commit to ooni/probe-cli that referenced this issue Feb 12, 2024
I realized it was not the best idea to modify the measurement algorithm
for producing a better test suite. With a small refactor, I can make it
such we only perform these changes when we need. Then, I need to
regenerate all the files, which comes with some churn. At this point,
it's sad but also fine. I'll try to improve things a bit more from now
one.

Part of ooni/probe#2677
bassosimone added a commit to ooni/probe-cli that referenced this issue Feb 12, 2024
We use scope for endpoint IDs. This helps to reduce churn when running
`./script/updateminipipeline.bash`.

Part of ooni/probe#2677
bassosimone added a commit to ooni/probe-cli that referenced this issue Feb 12, 2024
Using a singleton makes tests non-deterministic. Instead use an instance
for each measurer.

Helps with ooni/probe#2677.
bassosimone added a commit to ooni/probe-cli that referenced this issue Feb 12, 2024
@bassosimone bassosimone added the techdebt This issue describes technical debt label Feb 12, 2024
bassosimone added a commit to ooni/probe-cli that referenced this issue Feb 12, 2024
Zero out times and zero LTE measurement results not used by
minipipeline.

Helps with ooni/probe#2677.
bassosimone added a commit to ooni/probe-cli that referenced this issue Feb 12, 2024
bassosimone added a commit to ooni/probe-cli that referenced this issue Feb 13, 2024
It seems unnecessary in light of what we're currently testing. By doing
this, we allow for much smaller commits.

Tangentially related to ooni/probe#2677.
bassosimone added a commit to ooni/probe-cli that referenced this issue Feb 13, 2024
Arguably IDs starting from 10k for getaddrinfo more obviously have a
scope.

Related to ooni/probe#2677.
bassosimone added a commit to ooni/probe-cli that referenced this issue Feb 13, 2024
We just need a single IP address in both cases. Helps with
ooni/probe#2677.
bassosimone added a commit to ooni/probe-cli that referenced this issue Feb 13, 2024
This diff removes the remaining causes of churn in qatool (modulo flaky
tests).

Closes ooni/probe#2677.
Murphy-OrangeMud pushed a commit to Murphy-OrangeMud/probe-cli that referenced this issue Feb 13, 2024
We add dns.nextdns.io to the QA suite, such that we don't get a wrong
NXDOMAIN when trying to use it as the resolver.

We optionally sort the test keys to ensure there's less churn in diffs
produced when regenerating minipipeline test data.

We fix ./script/updateminipipeline.bash to use ./script/go.bash for
building as opposed to using go.

Also, include endpoint information in `tls_handshake_{start,stop}` and
`http_transaction_{start,stop}` events such that they are sorted
correctly when we sort events to reduce churn. This is necessary because
we're using the endpoint information to sort the network events.

Part of ooni/probe#2677; the churn is still
too high, and I need more changes.
Murphy-OrangeMud pushed a commit to Murphy-OrangeMud/probe-cli that referenced this issue Feb 13, 2024
I realized it was not the best idea to modify the measurement algorithm
for producing a better test suite. With a small refactor, I can make it
such we only perform these changes when we need. Then, I need to
regenerate all the files, which comes with some churn. At this point,
it's sad but also fine. I'll try to improve things a bit more from now
one.

Part of ooni/probe#2677
Murphy-OrangeMud pushed a commit to Murphy-OrangeMud/probe-cli that referenced this issue Feb 13, 2024
We use scope for endpoint IDs. This helps to reduce churn when running
`./script/updateminipipeline.bash`.

Part of ooni/probe#2677
Murphy-OrangeMud pushed a commit to Murphy-OrangeMud/probe-cli that referenced this issue Feb 13, 2024
…#1508)

Using a singleton makes tests non-deterministic. Instead use an instance
for each measurer.

Helps with ooni/probe#2677.
Murphy-OrangeMud pushed a commit to Murphy-OrangeMud/probe-cli that referenced this issue Feb 13, 2024
Murphy-OrangeMud pushed a commit to Murphy-OrangeMud/probe-cli that referenced this issue Feb 13, 2024
Zero out times and zero LTE measurement results not used by
minipipeline.

Helps with ooni/probe#2677.
Murphy-OrangeMud pushed a commit to Murphy-OrangeMud/probe-cli that referenced this issue Feb 13, 2024
Murphy-OrangeMud pushed a commit to Murphy-OrangeMud/probe-cli that referenced this issue Feb 13, 2024
It seems unnecessary in light of what we're currently testing. By doing
this, we allow for much smaller commits.

Tangentially related to ooni/probe#2677.
Murphy-OrangeMud pushed a commit to Murphy-OrangeMud/probe-cli that referenced this issue Feb 13, 2024
Arguably IDs starting from 10k for getaddrinfo more obviously have a
scope.

Related to ooni/probe#2677.
Murphy-OrangeMud pushed a commit to Murphy-OrangeMud/probe-cli that referenced this issue Feb 13, 2024
We just need a single IP address in both cases. Helps with
ooni/probe#2677.
Murphy-OrangeMud pushed a commit to Murphy-OrangeMud/probe-cli that referenced this issue Feb 13, 2024
This diff removes the remaining causes of churn in qatool (modulo flaky
tests).

Closes ooni/probe#2677.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working funder/otffoss2023-2024 techdebt This issue describes technical debt
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant