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

jsonapi, probeservices: refactoring before detecting blocking #602

Merged
merged 6 commits into from May 19, 2020

Conversation

bassosimone
Copy link
Member

This PR contains a but of unrelated diffs that perform refactoring of jsonapi and probeservices. I want these changes to be in before continuing with ooni/probe#887.

Part of ooni/probe#887. I determined that
we needed some refactoring of jsonapi before proceeding.
There is no reason in doing so. Mutable state leads to code that is
more difficult to analyse, hence, let's just do that.

Part of the refactoring that I determined was necessary before
moving ooni/probe#887 forward.
Closes #270

Part of the refactoring that I think it's necessary to do before
moving ooni/probe#887 forward.
This is part of ooni/probe#887 where I
determined that we needed to do some refactoring of jsonapi.

Specifically, here the goal is to avoid using magic functions as well
as to make sure tests check the properties we care about.
Instead, use embedding, so that we don't need to write tests
asserting that we pass downstream the correct fields.

Somewhere else (session? experiment?) we probably need to put
more tests, but at least here's one less middleman.

Part of the refactoring that I started such that it is easy
to do ooni/probe#887.
Keep returning a pointer in OpenReport. Apart from that, do not return
pointers and make sure structures cannot change themselves.

This should conclude the preliminary refactoring that I envisioned
before doing ooni/probe#887.
@bassosimone
Copy link
Member Author

Merging! (The failure seems to be a false positive and also tests pass locally)

@bassosimone bassosimone merged commit f888c5c into master May 19, 2020
@bassosimone bassosimone deleted the probe/887 branch May 19, 2020 13:15
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.

None yet

1 participant