feat: Add end to end tests for RAG-enabled chat using user-supplied documents - #2948
feat: Add end to end tests for RAG-enabled chat using user-supplied documents#2948anastasds wants to merge 5 commits into
Conversation
dmartinol
left a comment
There was a problem hiding this comment.
How to set this up as an optional CI check?
What do you mean?
The goal is for this set of tests to be able to be triggered on a PR involving changes to experimental features and then be required for merge - but to not run automatically for PRs so that experimental feature work never blocks other work. I'm not clear on what needs to be done to set that up. |
check if there is a linked issue with an 'experimental' label? (there are actions to get the linked issues, like https://github.com/kin/gh-action-get-linked-issues) |
|
There's an existing |
|
@nathan-weinberg I wasn't sure what all uses it and wanted to keep experimental features totally cordoned off, at least to start with, in order to avoid issues with experimental features from potentially blocking anything else. Happy to merge them in; I was being risk avoidant since this whole experimental features pattern seems to be net new. |
|
This pull request has merge conflicts that must be resolved before it can be |
|
@anastasds yes please use that script, it should mesh well with the mission of us keeping it: https://github.com/instructlab/instructlab/blob/main/docs/ci.md#end-to-end-e2e-tests Only |
|
@nathan-weinberg will do today |
9864450 to
d26735b
Compare
nathan-weinberg
left a comment
There was a problem hiding this comment.
Looking good to me overall - when this is out of draft are you able to trigger a test custom job @anastasds so we can see it in action?
1c5428e to
0e93434
Compare
| mkdir -p "${E2E_LOG_DIR}" | ||
|
|
||
| # set appropriate feature gating flag | ||
| if [ "$DEV_PREVIEW_FEATURE_GATING" -eq 1 ]; then |
There was a problem hiding this comment.
Note: you probably have these env vars in your shell, so you're not seeing the DEV_PREVIEW_FEATURE_GATING: unbound variable error.
Since we have set -u at the beginning, we have to ensure that the env variables are defined.
We can use either -n or provide a default as in:
if [ ${DEV_PREVIEW_FEATURE_GATING:-} -eq 1 ]; then
Explanation of set -u:
Treat unset variables and parameters other than the special parameters ‘@’ or ‘*’, or array variables subscripted with ‘@’ or ‘*’, as an error when performing parameter expansion. An error message will be written to the standard error, and a non-interactive shell will exit.
There was a problem hiding this comment.
I see now there are multiple references, so we can initialize the defaults after the getops loop or anywhere you prefer:
DEV_PREVIEW_FEATURE_GATING=${DEV_PREVIEW_FEATURE_GATING:-}
TECH_PREVIEW_FEATURE_GATING=${TECH_PREVIEW_FEATURE_GATING:-}
CUSTOM_FEATURE_GATING=${CUSTOM_FEATURE_GATING:-}|
This pull request has merge conflicts that must be resolved before it can be |
|
E2E (Custom) (instance-type: g6e.24xlarge, e2e_script_args: -dm) workflow launched on this PR: View run |
|
e2e workflow failed on this PR: View run, please investigate. |
|
E2E (Custom) (instance-type: g6e.12xlarge, e2e_script_args: -dm) workflow launched on this PR: View run |
|
e2e workflow failed on this PR: View run, please investigate. |
|
@Mergifyio rebase |
|
to be safe ^ |
Signed-off-by: Anastas Stoyanovsky <astoyano@redhat.com>
Signed-off-by: Daniele Martinoli <dmartino@redhat.com>
✅ Branch has been successfully rebased |
|
E2E (Custom) (instance-type: g6e.12xlarge, e2e_script_args: -dm) workflow launched on this PR: View run |
|
e2e workflow failed on this PR: View run, please investigate. |
Signed-off-by: Anastas Stoyanovsky <astoyano@redhat.com>
Signed-off-by: Anastas Stoyanovsky <astoyano@redhat.com>
|
E2E (Custom) (instance-type: g6e.12xlarge, e2e_script_args: -dm) workflow launched on this PR: View run |
|
e2e workflow failed on this PR: View run, please investigate. |
Signed-off-by: Anastas Stoyanovsky <astoyano@redhat.com>
|
This pull request has been automatically marked as stale because it has not had activity within 60 days. It will be automatically closed if no further activity occurs within 30 days. |
|
This pull request has been automatically closed due to inactivity. Please feel free to reopen if you intend to continue working on it! |
#2947
Depends on #2903, #2886
Run as
e2e-custom.sh -dmtest-data/raw_documentsrepoe2e-custom.shto accommodate dev preview functionality