Skip to content
This repository was archived by the owner on Mar 11, 2026. It is now read-only.
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,8 @@
"@types/extend": "^3.0.0",
"@types/is": "0.0.21",
"@types/mocha": "^5.2.5",
"@types/mv": "^2.1.0",
"@types/ncp": "^2.0.1",
"@types/nock": "^9.3.0",
"@types/on-finished": "^2.3.1",
"@types/pify": "^3.0.2",
Expand All @@ -105,6 +107,8 @@
"jsdoc-baseline": "git+https://github.com/hegemonic/jsdoc-baseline.git",
"linkinator": "^1.1.2",
"mocha": "^6.0.0",
"mv": "^2.1.1",
"ncp": "^2.0.0",
"nock": "^10.0.1",
"nyc": "^13.0.1",
"power-assert": "^1.6.0",
Expand Down
4 changes: 3 additions & 1 deletion src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,9 @@ export type DeleteResponse = google.protobuf.Empty;
export type LogSink = google_config.logging.v2.ILogSink;

export interface CreateSinkRequest {
destination: Bucket|Dataset|Topic|string;
// destination: Bucket|Dataset|Topic|string;
// tslint:disable-next-line no-any
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

@JustinBeckwith can you add a comment here indicating why any is being used, and what would be an ideal type definition if it were not for the npm/typescript limitations.

destination: any;
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

aha, this is the same thing I did in my local testing; glad that I'm figuring these things out.

filter?: string;
includeChildren?: boolean;
name?: string;
Expand Down
23 changes: 23 additions & 0 deletions system-test/fixtures/sample/package.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
{
"name": "logging-sample-fixture",
"description": "An app we're using to test the library.",
"scripts": {
"check": "gts check",
"clean": "gts clean",
"compile": "tsc -p .",
"fix": "gts fix",
"prepare": "npm run compile",
"pretest": "npm run compile",
"posttest": "npm run check",
"start": "node build/src/index.js"
},
"license": "Apache-2.0",
"dependencies": {
"@google-cloud/logging": "file:./logging.tgz"
},
"devDependencies": {
"@types/node": "^10.3.0",
"typescript": "^3.0.0",
"gts": "^0.9.0"
}
}
6 changes: 6 additions & 0 deletions system-test/fixtures/sample/src/index.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
import {Logging} from '@google-cloud/logging';
async function main() {
const logging = new Logging();
console.log(logging);
}
main();
10 changes: 10 additions & 0 deletions system-test/fixtures/sample/tsconfig.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
{
"extends": "./node_modules/gts/tsconfig-google.json",
"compilerOptions": {
"rootDir": ".",
"outDir": "build"
},
"include": [
"src/*.ts"
]
}
56 changes: 56 additions & 0 deletions system-test/install.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,56 @@
/**
* Copyright 2019 Google LLC. All Rights Reserved.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

import * as execa from 'execa';
import * as mv from 'mv';
import {ncp} from 'ncp';
import * as tmp from 'tmp';
import {promisify} from 'util';

const keep = false;
const mvp = promisify(mv) as {} as (...args: string[]) => Promise<void>;
const ncpp = promisify(ncp);
const stagingDir = tmp.dirSync({keep, unsafeCleanup: true});
const stagingPath = stagingDir.name;
const pkg = require('../../package.json');

describe('📦 pack and install', () => {
/**
* Create a staging directory with temp fixtures used to test on a fresh
* application.
*/
it('should be able to use the d.ts', async () => {
await execa('npm', ['pack', '--unsafe-perm']);
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

I like this approach to integration testing the TypeScript module, I wonder if this could be be abstracted into a module.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

@JustinBeckwith I see that you chose not to use post-install-check. I am guessing that there are things in that module that you don't like. Can you work with @DominicKramer and provide feedback to make sure we are serving your needs and making that module better for everyone ❤️.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Oh, I wasn't trying to be a :trollface:, I didn't realize this was essentially what post-install-check does.

It would be nice to eventually consolidate on a module like post-install-check ... I honestly don't love the name post-install-check if what it does is check that TypeScript has the appropriate dependencies. I read post-install-check and assumed it was something more abstract (a generic way of checking your packages post-install scripts?). I'd advocate a name like check-installed-types?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Yep, it is not the best name. We can rename it. How about something like ts-pack-n-play?

/cc @googleapis/node-team FYI

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

FWIW - I love that name. @ofrobots wanna open an issue over there to consider the rename?

const tarball = `google-cloud-logging-${pkg.version}.tgz`;
await mvp(tarball, `${stagingPath}/logging.tgz`);
await ncpp('system-test/fixtures/sample', `${stagingPath}/`);
await execa(
'npm', ['install', '--unsafe-perm'],
{cwd: `${stagingPath}/`, stdio: 'inherit'});
await execa(
'node', ['--throw-deprecation', 'build/src/index.js'],
{cwd: `${stagingPath}/`, stdio: 'inherit'});
});

/**
* CLEAN UP - remove the staging directory when done.
*/
after('cleanup staging', () => {
if (!keep) {
stagingDir.removeCallback();
}
});
});