-
Notifications
You must be signed in to change notification settings - Fork 1.7k
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
add type checking sample to sample tests #1129
add type checking sample to sample tests #1129
Conversation
…check_notebook_result script to not validate the pipeline runs when experiment arg is not provided
resolving #974 |
AFAIK all current type checking code can be tested by unit-tests that do not require cluster. |
Correct. It does not require the cluster. By having the sample test results shown in the same place, it is easier to check the result. |
And, the sample notebook is converted to python and get executed. |
Don't you get the Travis test results much sooner? They're also less flaky. I just want us to test as much as possible using local unit tests so that the contributors can easily run them after local changes. |
We can always add unit tests for the static type checking but it does not mean we can drop the sample tests here. The sample test here guarantees all samples work correctly. |
Of course. I did not mean to drop them. We should test the samples. I'm just asking to run the test locally, not on cluster (using Travis, for example). |
OK, let's LGTM this one. /lgtm |
I do not see the benefits of introducing the Travis-based sample tests into the sample test infra now.
|
/approve |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: gaoning777 The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
1 similar comment
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: gaoning777 The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
That's what I meant. |
Got it. Cannot agree more. |
This change is