Skip to content
This repository has been archived by the owner on Nov 16, 2023. It is now read-only.

test: support multi version node cases #121

Merged

Conversation

fs-eire
Copy link
Contributor

@fs-eire fs-eire commented Apr 3, 2019

This change update script tools/build.ts to copy v7-v10 node test cases and allow different node test version for test runner.

BrowserStack builds requires #120 as dependency to pass

[['v7', '5af210ca8a1c73aa6bae8754c9346ec54d0a756e'], // rel-1.2.3
['v8', '5cc146270945f0d007b140fb59a892a60ba69f49'], // rel-1.3.0
['v9', '4e67414849122d3df78bd72cee5717f90e715d12'], // rel-1.4.1
['v10', 'b22041c3f16cf8bcca9ed93982d6ffdf6ebf3746'], // master
Copy link
Member

Choose a reason for hiding this comment

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

Is the general idea to freeze a commit id after every onnx release ? For example after onnx locks down v10, we freeze the commit id for that and we keep rolling forward the commit id of master (for v11, and so on) ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yes that is the idea.

// The default backends and opset version lists. Those will be used in suite tests.
const DEFAULT_BACKENDS: ReadonlyArray<TestRunnerCliArgs.Backend> =
args.env === 'node' ? ['cpu', 'wasm'] : ['cpu', 'wasm', 'webgl'];
const DEFAULT_OPSET_VERSIONS: ReadonlyArray<number> = [10, 9, 8, 7];
Copy link
Member

Choose a reason for hiding this comment

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

Do we need to keep upgrading this after each onnx release too ? If so, I think we better document this...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

the answer is 'yes' ideally. but considering the fact that node test cases are rarely modified ( usually the change is to add cases to that folder), we can do this as demand.

there is another option: only support the stable ONNX version. This means we should only support v10 after ONNX v1.5 release (which will happen very soon though). thus we only update when a ONNX release happen

@fs-eire fs-eire merged commit e1fe3a9 into microsoft:master Apr 11, 2019
@fs-eire fs-eire deleted the test-node-test-allow-different-version branch April 11, 2019 21:05
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants