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

fix(webconnectivitylte): never sort test keys #1506

Merged
merged 3 commits into from
Feb 12, 2024
Merged

Conversation

bassosimone
Copy link
Contributor

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

After additional reflection, I realized it's not the right strategy
for us to sort the test keys conditionally on a global variable as part
of running the experiment. Rather, we should instead only perform this
kind of transformation when we're about to save them to disk.
It would obviously contain churn, and I would obviously need to
eyeball this a bit, unfortunately, but at this point we can only
proceed piecemeal with small improvements.
@bassosimone bassosimone requested a review from hellais as a code owner February 12, 2024 17:54
@bassosimone bassosimone merged commit 80b1681 into master Feb 12, 2024
25 checks passed
@bassosimone bassosimone deleted the issue/2677b branch February 12, 2024 18:06
Murphy-OrangeMud pushed a commit to Murphy-OrangeMud/probe-cli that referenced this pull request 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
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.

1 participant