Skip to content
This repository has been archived by the owner on Sep 28, 2018. It is now read-only.

Ensure user packages are separated from bundled packages #37

Merged
merged 5 commits into from Oct 31, 2017
Merged

Ensure user packages are separated from bundled packages #37

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
e22dd13
Use atom.packages.getPackageDirPaths to check if the package is bundled
Ben3eeE Oct 30, 2017
8cb8c4b
Remove path as a dependency
Ben3eeE Oct 30, 2017
2992a39
Change andCallFake to andReturn
Ben3eeE Oct 31, 2017
5cd0fe7
Remove the check for isBundledPackage after review
Ben3eeE Oct 31, 2017
4d3426c
More review feedback
Ben3eeE Oct 31, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
4 changes: 3 additions & 1 deletion lib/reporter.js
View file
Open in desktop
Expand Up @@ -4,6 +4,7 @@ import _ from 'underscore-plus'
import os from 'os'
import stackTrace from 'stack-trace'
import fs from 'fs-plus'
import path from 'path'

const API_KEY = '7ddca14cb60cbd1cd12d1b252473b076'
const LIB_VERSION = require('../package.json')['version']
Expand Down Expand Up @@ -177,11 +178,12 @@ export default class Reporter {

addPackageMetadata (error) {
let activePackages = atom.packages.getActivePackages()
const availablePackagePaths = atom.packages.getPackageDirPaths()
if (activePackages.length > 0) {
let userPackages = {}
let bundledPackages = {}
for (let pack of atom.packages.getActivePackages()) {
if (/\/app\.asar\//.test(pack.path)) {
if (!availablePackagePaths.includes(path.dirname(pack.path))) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To avoid negation you could just flip the if statements around :).

bundledPackages[pack.name] = pack.metadata.version
} else {
userPackages[pack.name] = pack.metadata.version
Expand Down
12 changes: 10 additions & 2 deletions spec/reporter-spec.js
View file
Open in desktop
Expand Up @@ -161,14 +161,18 @@ describe("Reporter", () => {
expect(error.privateMetadataDescription).toBeUndefined()
expect(error.metadata).toEqual({foo: "bar"});});})

it("adds bundled and user packages to the error's metadata", () => {
it('treats packages located in atom.packages.getPackageDirPaths as user packages', () => {
mockActivePackages = [
{name: 'user-1', path: '/Users/user/.atom/packages/user-1', metadata: {version: '1.0.0'}},
{name: 'user-2', path: '/Users/user/.atom/packages/user-2', metadata: {version: '1.2.0'}},
{name: 'bundled-1', path: '/Applications/Atom.app/Contents/Resources/app.asar/node_modules/bundled-1', metadata: {version: '1.0.0'}},
{name: 'bundled-2', path: '/Applications/Atom.app/Contents/Resources/app.asar/node_modules/bundled-2', metadata: {version: '1.2.0'}},
]

const packageDirPaths = ['/Users/user/.atom/packages']
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It may be worth it in a follow-up PR to convert all of these hardcoded paths to use path.join to ensure that everything works correctly on Windows as well.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍 I can do that


spyOn(atom.packages, 'getPackageDirPaths').andReturn(packageDirPaths)

let error = new Error()
Error.captureStackTrace(error)
reporter.reportUncaughtException(error)
Expand Down Expand Up @@ -360,14 +364,18 @@ describe("Reporter", () => {
})
})

it("adds bundled and user packages to the error's metadata", () => {
it('treats packages located in atom.packages.getPackageDirPaths as user packages', () => {
mockActivePackages = [
{name: 'user-1', path: '/Users/user/.atom/packages/user-1', metadata: {version: '1.0.0'}},
{name: 'user-2', path: '/Users/user/.atom/packages/user-2', metadata: {version: '1.2.0'}},
{name: 'bundled-1', path: '/Applications/Atom.app/Contents/Resources/app.asar/node_modules/bundled-1', metadata: {version: '1.0.0'}},
{name: 'bundled-2', path: '/Applications/Atom.app/Contents/Resources/app.asar/node_modules/bundled-2', metadata: {version: '1.2.0'}},
]

const packageDirPaths = ['/Users/user/.atom/packages']

spyOn(atom.packages, 'getPackageDirPaths').andReturn(packageDirPaths)

let error = new Error()
Error.captureStackTrace(error)
reporter.reportFailedAssertion(error)
Expand Down