Skip to content

Commit

Permalink
Update report path to match new package structure
Browse files Browse the repository at this point in the history
In PR #545 we've merged the nodejs-ext package into the nodejs package.
This change moved some files around, and moved the extension install
scripts one level deeper into the `extension` subdirectory. Update the
report path name generation to match the new depth of the file.

Previously: `packages/nodejs-ext/scripts/extension.js`
New: `packages/nodejs/scripts/extension/extension.js`

It's interesting that no tests failed because of this. I think it's not
caught in the diagnose_tests, because we stub the install report.

I've now added a simple test to test the presence of the report, and not
a parsing error that occurs when no file could be read or there was a
problem reading the file. I'm struggling with Jest's mock system to
write a test for this and I'd rather look at it when addressing issue
#550 that updates the path location, which makes it easier to stub I
hope.
  • Loading branch information
tombruijn committed Jan 18, 2022
1 parent 58f1ae0 commit 854390c
Show file tree
Hide file tree
Showing 3 changed files with 21 additions and 1 deletion.
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
---
bump: "patch"
type: "fix"
---

Fix install report path name generation. This fixes the diagnose report not being able to find the path where the install report is stored.
2 changes: 1 addition & 1 deletion packages/nodejs/scripts/extension/report.js
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,7 @@ function createDownloadReport(report) {
function reportPath() {
// Navigate up to the app dir. Move up the scripts dir, package dir,
// @appsignal dir and node_modules dir.
const appPath = path.join(__dirname, "../../../../")
const appPath = path.join(__dirname, "../../../../../")
const hash = crypto.createHash("sha256")
hash.update(appPath)
const reportPathDigest = hash.digest("hex")
Expand Down
14 changes: 14 additions & 0 deletions packages/nodejs/src/__tests__/diagnose.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,20 @@ describe("DiagnoseTool", () => {
expect(output.process.uid).toEqual(process.getuid())
})

describe("install report", () => {
it("fetches the install report", async () => {
const output = await tool.generate()

const install = output.installation
expect(install).not.toHaveProperty("parsing_error")
expect(install).toHaveProperty("build")
expect(install).toHaveProperty("download")
expect(install).toHaveProperty("host")
expect(install).toHaveProperty("language")
expect(install).toHaveProperty("result")
})
})

it("returns the log_dir_path", async () => {
const report = await tool.generate()
expect(report.paths.log_dir_path.path).toEqual("/tmp")
Expand Down

0 comments on commit 854390c

Please sign in to comment.