Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update conformance testee for @connectrpc/connect-web #1183

Merged
merged 1 commit into from
Aug 26, 2024
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
19 changes: 19 additions & 0 deletions .github/workflows/web-conformance.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -57,3 +57,22 @@ jobs:
${{ runner.os }}-connect-web-safari-conformance-
- name: testwebsafariconformance
run: make testwebsafariconformance
node:
runs-on: ubuntu-22.04
Comment on lines +60 to +61
Copy link
Member Author

Choose a reason for hiding this comment

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

We're running conformance tests for @connectrpc/connect-web on Node.js now. They run quick enough, seems worth it.

steps:
- name: checkout
uses: actions/checkout@v4
- uses: actions/setup-node@v4
with:
node-version-file: .nvmrc
- name: cache
uses: actions/cache@v4
with:
path: |
~/.tmp
.tmp
key: ${{ runner.os }}-connect-web-node-conformance-${{ hashFiles('Makefile') }}
restore-keys: |
${{ runner.os }}-connect-web-node-conformance-
- name: testwebnodeconformance
run: make testwebnodeconformance
5 changes: 2 additions & 3 deletions .gitignore
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
/.tmp
/packages/*/dist
/packages/connect-web-test/local.log
/packages/connect-node-test/.connect-node-h1-server.*
/packages/connect-web/conformance/logs/*
node_modules
.wrangler
.wrangler
19 changes: 11 additions & 8 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@ NODE18_VERSION ?= v18.16.0
NODE16_VERSION ?= v16.20.0
NODE_OS = $(subst Linux,linux,$(subst Darwin,darwin,$(shell uname -s)))
NODE_ARCH = $(subst x86_64,x64,$(subst aarch64,arm64,$(shell uname -m)))
CONFORMANCE_BROWSER ?= chrome

node_modules: package-lock.json
npm ci
Expand Down Expand Up @@ -205,23 +204,27 @@ testconnectfastifyconformance: $(BUILD)/connect-fastify
npm run -w packages/connect-fastify conformance

.PHONY: testwebconformance
testwebconformance: testwebchromeconformance testwebfirefoxconformance testwebsafariconformance
testwebconformance: testwebchromeconformance testwebfirefoxconformance testwebsafariconformance testwebnodeconformance

.PHONY: testwebchromeconformance
testwebchromeconformance: $(BUILD)/connect-web
npm run -w packages/connect-web conformance:client:chrome
npm run -w packages/connect-web conformance:client:chrome:promise
npm run -w packages/connect-web conformance:client:chrome:callback

.PHONY: testwebfirefoxconformance
testwebfirefoxconformance: $(BUILD)/connect-web
npm run -w packages/connect-web conformance:client:firefox
npm run -w packages/connect-web conformance:client:firefox:promise
npm run -w packages/connect-web conformance:client:firefox:callback

.PHONY: testwebsafariconformance
testwebsafariconformance: $(BUILD)/connect-web
npm run -w packages/connect-web conformance:client:safari
npm run -w packages/connect-web conformance:client:safari:promise
npm run -w packages/connect-web conformance:client:safari:callback

.PHONY: testwebconformancelocal
Copy link
Member Author

Choose a reason for hiding this comment

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

This didn't work as documented. Since the npm script runs two commands with &&, the --browser flag was only appended to the second command. The replacement is explained in packages/connect-conformance/README.md

testwebconformancelocal: $(BUILD)/connect-conformance
npm run -w packages/connect-web conformance:client:browser -- --browser $(CONFORMANCE_BROWSER)
.PHONY: testwebnodeconformance
testwebnodeconformance: $(BUILD)/connect-web
npm run -w packages/connect-web conformance:client:node:promise
npm run -w packages/connect-web conformance:client:node:callback

.PHONY: testcloudflareconformance
testcloudflareconformance: $(BUILD)/connect-conformance $(BUILD)/connect-node
Expand Down
53 changes: 28 additions & 25 deletions packages/connect-conformance/src/conformance.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ import {
chmodSync,
mkdirSync,
} from "node:fs";
import { join as joinPath } from "node:path";
import { join as joinPath, basename } from "node:path";
import { unzipSync, gunzipSync } from "fflate";
import * as tar from "tar-stream";
import { pipeline } from "node:stream/promises";
Expand All @@ -34,15 +34,18 @@ const [, version] = /conformance:(v\d+\.\d+\.\d+)/.exec(scripts.generate) ?? [
"?",
];

const name = "connectconformance";
const downloadUrl = `https://github.com/connectrpc/conformance/releases/download/${version}`;

export async function run() {
const { archive, bin } = getArtifactNameForEnv();
const tempDir = getTempDir();
const artifactName = getArtifactNameForEnv();
const assetPath = joinPath(tempDir, artifactName);
await download(`${downloadUrl}/${artifactName}`, assetPath);
execFileSync(await extractBin(assetPath), process.argv.slice(2), {
const binPath = joinPath(tempDir, bin);
if (!existsSync(binPath)) {
const archivePath = joinPath(tempDir, archive);
await download(`${downloadUrl}/${archive}`, archivePath);
await extractBin(archivePath, binPath);
}
execFileSync(binPath, process.argv.slice(2), {
stdio: "inherit",
});
}
Expand All @@ -58,31 +61,28 @@ async function download(url: string, path: string) {
writeFileSync(path, new Uint8Array(await res.arrayBuffer()));
}

async function extractBin(path: string) {
if (path.endsWith(".zip")) {
const unzipped = unzipSync(readFileSync(path), {
async function extractBin(archivePath: string, binPath: string) {
Copy link
Member Author

Choose a reason for hiding this comment

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

Running the connectconformance command in parallel used to fail without an error message, because this script tried to write to the file that was already running.

const binName = basename(binPath);
if (archivePath.endsWith(".zip")) {
const unzipped = unzipSync(readFileSync(archivePath), {
filter(file) {
return file.name === "connectconformance.exe";
return file.name === binName;
},
});
const binBytes = unzipped["connectconformance.exe"] as
| Uint8Array
| undefined;
const binBytes = unzipped[binName] as Uint8Array | undefined;
if (binBytes === undefined) {
throw new Error("Failed to extract connectconformance.exe");
throw new Error(`Failed to extract ${binName}`);
}
const bin = joinPath(getTempDir(), "connectconformance.exe");
writeFileSync(bin, binBytes);
return bin;
writeFileSync(binPath, binBytes);
}
const bin = joinPath(getTempDir(), "connectconformance");
const extract = tar.extract();
extract.on("entry", (header, stream, next) => {
if (header.name === "connectconformance") {
if (header.name === binName) {
const chunks: Buffer[] = [];
stream.on("data", (chunk: Buffer) => chunks.push(chunk));
stream.on("end", () => {
writeFileSync(bin, Buffer.concat(chunks));
writeFileSync(binPath, Buffer.concat(chunks));
chmodSync(binPath, 0o755);
next();
});
} else {
Expand All @@ -93,14 +93,12 @@ async function extractBin(path: string) {
await pipeline(
new Readable({
read() {
this.push(gunzipSync(readFileSync(path)));
this.push(gunzipSync(readFileSync(archivePath)));
this.push(null);
},
}),
extract,
);
chmodSync(bin, 755);
return bin;
}

function getTempDir() {
Expand All @@ -111,9 +109,10 @@ function getTempDir() {
return tempDir;
}

function getArtifactNameForEnv() {
function getArtifactNameForEnv(): { archive: string; bin: string } {
let build = "";
let ext = ".tar.gz";
let bin = "connectconformance";
switch (os.platform()) {
case "darwin":
switch (os.arch()) {
Expand Down Expand Up @@ -141,6 +140,7 @@ function getArtifactNameForEnv() {
break;
case "win32":
ext = ".zip";
bin = "connectconformance.exe";
switch (os.arch()) {
case "arm64":
build = "Windows-arm64";
Expand All @@ -155,5 +155,8 @@ function getArtifactNameForEnv() {
default:
throw new Error(`Unsupported platform: ${os.platform()}`);
}
return `${name}-${version}-${build}${ext}`;
return {
archive: `connectconformance-${version}-${build}${ext}`,
bin,
};
}
1 change: 0 additions & 1 deletion packages/connect-conformance/src/promise-client.ts
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,6 @@ async function unary(
if (req.requestMessages.length !== 1) {
throw new Error("Unary method requires exactly one request message");
}
req.cancel;
const msg = req.requestMessages[0];
const uReq = idempotent ? new IdempotentUnaryRequest() : new UnaryRequest();
if (!msg.unpackTo(uReq)) {
Expand Down
29 changes: 15 additions & 14 deletions packages/connect-web/conformance/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,26 +6,27 @@ It uses the [conformance runner](https://github.com/connectrpc/conformance/relea

## Running conformance tests

## Using a headless browser

Run `make testwebconformance` to run all conformance tests in the following headless browsers / environments:
Tests run in the following environments:

* Chrome
* Firefox
* Node
* Safari (only if running in OSX. Safari requires users to enable the `Allow Remote Automation` option in Safari's Develop menu)
* Safari (only if running in OSX. Safari requires users to enable the "Allow Remote Automation" option in Safari's Develop menu)
* Node.js

The individual tests can also be run via npm:
For every environment, two client flavors are available:
* Promise (using `createPromiseClient`)
* Callback (using `createCallbackClient`)

`npm run conformance:client:chrome`
`npm run conformance:client:firefox`
`npm run conformance:client:safari`
`npm run conformance:client:node`
For every combination, an npm script is available:

## Using a local browser
`npm run conformance:client:<chrome|firefox|safari|node>:<promise|callback>`

Run `make testwebconformancelocal` to run the tests in a local browser. This will open a Chrome browser and run the tests. If you want to run the tests in a different browser, set the `CONFORMANCE_BROWSER` environment variable.
Before you run npm scripts, make sure to build dependencies with `make .tmp/build/connect-web`.

## Using a local browser

Also available as an npm script:
To launch a browser window with access to the browser's network inspector, append the `--openBrowser` flag to the npm script:

`npm run conformance:client:browser`
```
npm run conformance:client:chrome:promise -- --openBrowser
```
19 changes: 8 additions & 11 deletions packages/connect-web/conformance/browserscript.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@
// See the License for the specific language governing permissions and
// limitations under the License.

import { ConnectError } from "@connectrpc/connect";
import {
invokeWithCallbackClient,
invokeWithPromiseClient,
Expand All @@ -31,8 +30,7 @@ declare global {
}
}

// The main entry point into the browser code running in Puppeteer/headless Chrome.
// This function is invoked by the page.evaluate call in grpcwebclient.
// This function is invoked by the browser.executeAsync call in client.ts
async function runTestCase(
data: number[],
useCallbackClient: boolean,
Expand All @@ -42,18 +40,17 @@ async function runTestCase(
testName: req.testName,
});
try {
let invoke;
if (useCallbackClient) {
invoke = invokeWithCallbackClient;
} else {
invoke = invokeWithPromiseClient;
}
const result = await invoke(createTransport(req), req);
const transport = createTransport(req);
const result = useCallbackClient
? await invokeWithCallbackClient(transport, req)
: await invokeWithPromiseClient(transport, req);
res.result = { case: "response", value: result };
} catch (err) {
res.result = {
case: "error",
value: new ClientErrorResult({ message: ConnectError.from(err).message }),
value: new ClientErrorResult({
message: `Failed to run test case: ${String(err)}`,
}),
Comment on lines +51 to +53
Copy link
Member Author

Choose a reason for hiding this comment

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

Wrapping the error isn't helpful here, this keeps more info.

};
}
return Array.from(res.toBinary());
Expand Down
Loading