-
Notifications
You must be signed in to change notification settings - Fork 48
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
Remove incorrect e2e implementation #76
Remove incorrect e2e implementation #76
Conversation
Signed-off-by: Oleg S <[email protected]>
6ee4eb3
to
ecc289e
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Why was spellcheck removed here? That has nothing to do with E2E |
Testing the user end-to-end instructlab workflow is the point of this workflow. It will give you an early indication if your changes to the library break the CLI. Sometimes that might be on purpose, but in case it's not, it will let you know before you publish a release. This is not intended to replace other functional testing aimed at the library more directly, but the full end-to-end workflow is important. I'm OK if you want to disable it for the moment, but it needs to go back on at some point. A better way to do that is to just edit the workflow to turn it off instead of removing it from the repo. |
This reverts commit ebc5e31 from instructlab#76. Here is my comment on instructlab#76 that better explains how this workflow fits in to the bigger picture: > Testing the user end-to-end instructlab workflow is the point of > this workflow. It will give you an early indication if your changes to > the library break the CLI. Sometimes that might be on purpose, but in > case it's not, it will let you know before you publish a release. > > This is not intended to replace other functional testing aimed at the > library more directly, but the full end-to-end workflow is important. > > I'm OK if you want to disable it for the moment, but it needs to go > back on at some point. A better way to do that is to just edit the > workflow to turn it off instead of removing it from the repo. Issue instructlab#63 Signed-off-by: Russell Bryant <[email protected]>
This reverts commit ebc5e31 from instructlab#76. Here is my comment on instructlab#76 that better explains how this workflow fits in to the bigger picture: > Testing the user end-to-end instructlab workflow is the point of > this workflow. It will give you an early indication if your changes to > the library break the CLI. Sometimes that might be on purpose, but in > case it's not, it will let you know before you publish a release. > > This is not intended to replace other functional testing aimed at the > library more directly, but the full end-to-end workflow is important. > > I'm OK if you want to disable it for the moment, but it needs to go > back on at some point. A better way to do that is to just edit the > workflow to turn it off instead of removing it from the repo. The previous commit also removed spellcheck (though it wasn't mentioned). I have left it out in this PR. Issue instructlab#63 Signed-off-by: Russell Bryant <[email protected]>
This reverts commit ebc5e31 from instructlab#76. Here is my comment on instructlab#76 that better explains how this workflow fits in to the bigger picture: > Testing the user end-to-end instructlab workflow is the point of > this workflow. It will give you an early indication if your changes to > the library break the CLI. Sometimes that might be on purpose, but in > case it's not, it will let you know before you publish a release. > > This is not intended to replace other functional testing aimed at the > library more directly, but the full end-to-end workflow is important. > > I'm OK if you want to disable it for the moment, but it needs to go > back on at some point. A better way to do that is to just edit the > workflow to turn it off instead of removing it from the repo. The previous commit also removed spellcheck (though it wasn't mentioned). I have left it out in this PR. Issue instructlab#63 Signed-off-by: Russell Bryant <[email protected]>
We're removing the end-to-end test because these rely on the implementation of ilab.
What we want instead is to have e2e tests that are based solely on the code of this repository, and not a separate tool consuming instructlab-training internally.
Fixes #74