Use json dumps in test fixtures instead of pipe dumps

RESOLVED FIXED

Status

Socorro
Webapp
RESOLVED FIXED
4 years ago
a month ago

People

(Reporter: peterbe, Assigned: is2ei)

Tracking

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [good first bug])

Attachments

(1 attachment)

53 bytes, text/x-github-pull-request
Details | Review | Splinter Review
(Reporter)

Description

4 years ago
Socorro no longer uses pipe dumps (e.g "OS|Mac OS X|10.6.8 10K549\\nCPU|amd64|family 6 mod|1") in the processors so the webapps will stop receiving these when it pulls from the middleware. Instead, it will receive JSON dumps (e.g. {"crash_info": {"address": "0x88", ....})

We should change our fixtures to make them slightly more realistic. Who knows, we might find some sneaky bugs we hadn't thought about. 

An example:
https://github.com/mozilla/socorro/blob/40abbb6cd50c86a82aa9c3122003a539c6494989/webapp-django/crashstats/crashstats/tests/test_views.py#L3162

What it should do:
https://github.com/mozilla/socorro/blob/master/webapp-django/crashstats/crashstats/tests/test_views.py#L3816
(Reporter)

Updated

4 years ago
Whiteboard: [good first bug]
(Assignee)

Comment 1

2 months ago
Hi,

Is this issue still available?

> What it should do:
> https://github.com/mozilla/socorro/blob/master/webapp-django/crashstats/crashstats/tests/test_views.py#L3816

It seems the file does not have the line now.
Flags: needinfo?(peterbe)
(Reporter)

Comment 2

2 months ago
Hi Issei,
I don't know what the status is of that. Or if it's important any more. 
There are still unit tests that use the pipe-dump format. E.g. https://github.com/mozilla-services/socorro/blob/e0fde4d244172a4b6ab00876564cfaaa898a2717/webapp-django/crashstats/crashstats/tests/test_views.py#L1320

Redirecting this to Will.
Flags: needinfo?(peterbe) → needinfo?(willkg)
Issei: Sure! If you want to work on this, that's fine by me. I don't know offhand how to find all the places in the test code that uses pipe dumps. You might have to search around.
Assignee: nobody → is2ei.horie
Flags: needinfo?(willkg)
(Assignee)

Updated

2 months ago
Status: NEW → ASSIGNED
(Reporter)

Comment 5

2 months ago
Without digging too deep in the details, I'm pretty confident that you can delete all references to the `parse_dump` function. I.e. remove this "else" block: https://github.com/mozilla-services/socorro/blob/40abbb6cd50c86a82aa9c3122003a539c6494989/webapp-django/crashstats/crashstats/views.py#L1058-L1063 and then you shouldn't need this "if" statement any more: https://github.com/mozilla-services/socorro/blob/40abbb6cd50c86a82aa9c3122003a539c6494989/webapp-django/crashstats/crashstats/views.py#L1045

In fact, if you start with that, you'll see which tests break and work backwards from that.
Flags: needinfo?(peterbe)
(Reporter)

Updated

2 months ago
Flags: needinfo?(willkg)
(Assignee)

Comment 6

a month ago
Created attachment 8984670 [details] [review]
GitHub PR

Thanks! I created a PR.

Comment 7

a month ago
Commits pushed to master at https://github.com/mozilla-services/socorro

https://github.com/mozilla-services/socorro/commit/1a7ff0dde5ecae891853425ca07c20c35db8645e
Bug 1091729: Use json dumps in test fixtures instead of pipe dumps

https://github.com/mozilla-services/socorro/commit/40eafdd47476bd0d3b076661d5b5094c7143bc48
Merge pull request #4476 from is2ei/bug-1091729

Bug 1091729: Use json dumps in test fixtures instead of pipe dumps
Thank you for working on this!
Status: ASSIGNED → RESOLVED
Last Resolved: a month ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.