Closed Bug 1091729 Opened 10 years ago Closed 6 years ago

Use json dumps in test fixtures instead of pipe dumps

Categories

(Socorro :: Webapp, task)

x86
macOS
task
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: peterbe, Assigned: is2ei)

Details

(Whiteboard: [good first bug])

Attachments

(1 file)

53 bytes, text/x-github-pull-request
Details | Review
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
Whiteboard: [good first bug]
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)
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)
Status: NEW → ASSIGNED
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)
Flags: needinfo?(willkg)
Attached file GitHub PR
Thanks! I created a PR.
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
Closed: 6 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: