Opened 11 months ago

Closed 8 days ago

#29259 closed task (fixed)

Ensure high test coverage for Snowflake

Reported by: ahf Owned by: cohosh
Priority: Medium Milestone:
Component: Circumvention/Snowflake Version:
Severity: Normal Keywords: anti-censorship-roadmap-september
Cc: dcf, arlolra, cohosh Actual Points:
Parent ID: Points:
Reviewer: Sponsor: Sponsor28-must

Description (last modified by cohosh)

We should aim for high test coverage and proper CI setup for Snowflake as early as possible in the process to ensure that we don't cause regressions or break things "randomly".

Known issues:

  • We don't currently have many proxy-go tests.
  • Tests in client/lib/rendezvous.go rely on specific HTTP response bodies which is prone to change and unnecessary

Child Tickets

TicketStatusOwnerSummaryComponent
#29489closedcohoshSet up automated local testing environment for SnowflakeCircumvention/Snowflake
#30867closedcohoshWrite proxy-go tests to cover existing implementationCircumvention/Snowflake
#30868closedModify client rendezvous library to remove hard-coded responsesCircumvention/Snowflake
#32300closedcohoshImprove snowflake server test coverageCircumvention/Snowflake

Attachments (5)

broker-coverage.html (39.4 KB) - added by cohosh 2 weeks ago.
client-coverage.html (11.8 KB) - added by cohosh 2 weeks ago.
client-lib-coverage.html (38.5 KB) - added by cohosh 2 weeks ago.
common-messages-coverage.html (8.0 KB) - added by cohosh 2 weeks ago.
common-safelog-coverage.html (4.8 KB) - added by cohosh 2 weeks ago.

Download all attachments as: .zip

Change History (22)

comment:1 Changed 8 months ago by cohosh

Description: modified (diff)

comment:2 Changed 7 months ago by cohosh

Description: modified (diff)

comment:3 Changed 7 months ago by cohosh

Owner: set to cohosh
Status: newassigned

comment:4 Changed 6 months ago by gaba

Keywords: ex-sponsor-19 added

Adding the keyword to mark everything that didn't fit into the time for sponsor 19.

comment:5 Changed 6 months ago by phw

Sponsor: Sponsor19Sponsor28-must

Moving from Sponsor 19 to Sponsor 28.

comment:6 Changed 6 months ago by cohosh

Description: modified (diff)

comment:7 Changed 6 months ago by arlolra

Since we have js and go implementations of the proxy, it might be nice to have a test runner that can exercise both in some language independent way. Just a thought.

comment:8 Changed 6 months ago by cohosh

Hm, this is an interesting idea, but it sounds like a lot more work. I suppose the goal of this would be to make sure that both versions show the same (or similar) behaviours?

The broker-facing functions (pollOffer and sendAnswer) and the proxying functions appear similarly structured across the two projects at least. We'd probably still want some independent tests for each implementation since they do differ a bit in how they work (e.g., the proxy-go instance uses tokens to handle multiple clients at once).

comment:9 Changed 6 months ago by arlolra

I suppose the goal of this would be to make sure that both versions show the same (or similar) behaviours?

Ya, that they comply with some sort of specification.

We'd probably still want some independent tests for each implementation since they do differ a bit in how they work (e.g., the proxy-go instance uses tokens to handle multiple clients at once).

That's only because it hasn't been implemented on the js side yet, see #25601

But yes, to your point, I was thinking more in terms of integration than unit testing.

comment:10 Changed 6 months ago by gaba

Keywords: anti-censorship-roadmap added

comment:11 Changed 5 months ago by arlolra

Milestone: Tor: unspecified
Version: Tor: unspecified

comment:12 Changed 5 months ago by gaba

Keywords: anti-censorship-roadmap-september added; ex-sponsor-19 anti-censorship-roadmap removed

comment:13 Changed 2 weeks ago by cohosh

Status: assignedneeds_review

Here are some test additions and touch-ups to existing go unit testing code: https://github.com/cohosh/snowflake/pull/16

Changed 2 weeks ago by cohosh

Attachment: broker-coverage.html added

Changed 2 weeks ago by cohosh

Attachment: client-coverage.html added

Changed 2 weeks ago by cohosh

Attachment: client-lib-coverage.html added

Changed 2 weeks ago by cohosh

Changed 2 weeks ago by cohosh

comment:14 Changed 2 weeks ago by cohosh

Okay I added a few code coverage visualizations. Obviously that's not the full story here, but I'm going to recommend calling this ticket done and then making another ticket that focuses on integration-style tests. This will be a bit tricker to do but I have some ideas of how to do it with snowbox. I do manual testing with snowbox before PRs and I have an idea on how to automate that. Since it's already in a docker container we can probably loop that into our CI.

comment:15 in reply to:  13 Changed 8 days ago by phw

Status: needs_reviewmerge_ready

Replying to cohosh:

Here are some test additions and touch-ups to existing go unit testing code: https://github.com/cohosh/snowflake/pull/16


Looks good to me!

comment:17 Changed 8 days ago by cohosh

Resolution: fixed
Status: merge_readyclosed
Note: See TracTickets for help on using tickets.