fix(ui): removed buildActiveQuery func to prevent queries from being reset when timeranges are selected. #16404
Conversation
ui/cypress/e2e/explorer.test.ts
Outdated
| @@ -349,8 +349,7 @@ describe('DataExplorer', () => { | |||
| }) | |||
| }) | |||
|
|
|||
| // todo: investigate flakiness of this test: https://github.com/influxdata/influxdb/issues/16330 | |||
| describe.skip('raw script editing', () => { | |||
| describe('raw script editing', () => { | |||
There was a problem hiding this comment.
i think the failures that were revealed with this action being dispatched are masking a deeper problem. i have another pr that is handling this: #16392.
did you run these e2e tests a few times to make sure they're reliable in ci? i'm kind of curious about the reliability of them. i could get this test to pass every time locally, and sometimes on ci.
There was a problem hiding this comment.
@hoorayimhelping I did run them a few times but I have no problem skipping them here in favor of your PR
There was a problem hiding this comment.
@asalem1 I think they're okay to be run now, I have what I think is the fix in master. I was more curious about the root cause of this issue - I'm having a hard time figuring out why removing buildActiveQuery had an effect on these tests and I was wondering if you found anything out in your digging
There was a problem hiding this comment.
@hoorayimhelping aw, I see. Truthfully, I'm not sure I fully comprehend why the test was failing when that line was added, and why it's no longer failing. I imagine that there are unintended consequences to the functionality of the utility function that may have been effecting the query builder / script editor that we are currently unaware of.
ghost
requested review from
…reset when timeranges are selected. Fixed broken test that was being skipped
asalem1
force-pushed
the
timerange-query-reset
branch
from
4f73817
to
277bb4e
Compare
asalem1
changed the title
ui/cypress/e2e/explorer.test.ts
Outdated
| @@ -349,7 +349,7 @@ describe('DataExplorer', () => { | |||
| }) | |||
| }) | |||
|
|
|||
| describe('raw script editing', () => { | |||
| describe.skip('raw script editing', () => { | |||
There was a problem hiding this comment.
| describe.skip('raw script editing', () => { | |
| describe('raw script editing', () => { |
|
@asalem1 this is good to merge after removing the |
…reset when timeranges are selected. (#16404) fix(ui): removed buildActiveQuery func to prevent queries from being reset when timeranges are selected. Fixed broken test that was being skipped
Closes #16369
Problem
Calling
buildActiveQueryafterSET_BUILDER_TAG_VALUESwas resetting the query whenever the timerange was being setSolution
The rationale for calling the
buildActiveQueryonSET_BUILDER_TAG_VALUESwas to ensure that:This seems to already be occurring whenever a query is drafted and is unnecessary. Since this change had originally broken an e2e explorer test, removing this line passes the test.
One key metric to ensure that this is in fact a stable solution would be to: