You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@superset.apache.org by "aminghadersohi (via GitHub)" <gi...@apache.org> on 2025/11/06 03:53:07 UTC

[PR] test: revert unrelated flaky test fix from MCP PR [superset]

aminghadersohi opened a new pull request, #36015:
URL: https://github.com/apache/superset/pull/36015

   ### SUMMARY
   
   Reverts an unrelated flaky test fix that was accidentally included in PR #35163 (MCP service infrastructure).
   
   ### ISSUE
   
   The MCP service PR (#35163) included a `time.sleep(0.01)` workaround for a flaky test in `tests/integration_tests/charts/commands_tests.py`. As noted in the PR review, this change was unrelated to the MCP service and should have been in a separate PR.
   
   ### CHANGES
   
   **Reverted**:
   - Removed `import time` statement
   - Removed `time.sleep(0.01)` line before timestamp comparison
   
   **File**: `tests/integration_tests/charts/commands_tests.py`
   
   ### MOTIVATION
   
   This flaky test fix deserves its own investigation and proper solution:
   - The timestamp race condition needs proper analysis
   - Solutions like mocking time with `freezegun` may be more appropriate
   - The fix should be accompanied by explanation of the root cause
   
   ### NEXT STEPS
   
   - [ ] Create GitHub issue to track proper flaky test investigation
   - [ ] Investigate why `last_saved_at` timestamp comparison is flaky
   - [ ] Implement proper fix (mock time, use freezegun, or fix UpdateChartCommand)
   - [ ] Add test documentation explaining the timing sensitivity
   
   ### TESTING
   
   ✅ Reverted changes only
   ✅ No functional changes to test behavior
   ✅ Test may become flaky again (expected, needs proper fix)
   
   ### ADDITIONAL INFORMATION
   
   - [ ] Has associated issue: Will create after revert
   - [x] Required feature flags: None
   - [ ] Changes UI: No
   - [ ] Includes DB Migration: No
   - [ ] Introduces new feature or API: No
   - [x] Removes existing feature or API: Yes (removes workaround)
   
   **References**:
   - Original PR: #35163


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


Re: [PR] test: revert unrelated flaky test fix from MCP PR [superset]

Posted by "bito-code-review[bot] (via GitHub)" <gi...@apache.org>.
bito-code-review[bot] commented on PR #36015:
URL: https://github.com/apache/superset/pull/36015#issuecomment-3494827521

   AI Code Review is in progress (usually takes 3 to 15 minutes unless it's a very large PR).


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


Re: [PR] test: revert unrelated flaky test fix from MCP PR [superset]

Posted by "sadpandajoe (via GitHub)" <gi...@apache.org>.
sadpandajoe merged PR #36015:
URL: https://github.com/apache/superset/pull/36015


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


Re: [PR] test: revert unrelated flaky test fix from MCP PR [superset]

Posted by "codecov[bot] (via GitHub)" <gi...@apache.org>.
codecov[bot] commented on PR #36015:
URL: https://github.com/apache/superset/pull/36015#issuecomment-3494852166

   ## [Codecov](https://app.codecov.io/gh/apache/superset/pull/36015?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) Report
   :white_check_mark: All modified and coverable lines are covered by tests.
   :white_check_mark: Project coverage is 48.48%. Comparing base ([`1f960d5`](https://app.codecov.io/gh/apache/superset/commit/1f960d5761f31d3cac2697a988b64ba08b2d980b?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)) to head ([`5cb2201`](https://app.codecov.io/gh/apache/superset/commit/5cb2201aa565cde1ba8e20cb36ede8e3422702f5?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)).
   
   <details><summary>Additional details and impacted files</summary>
   
   
   
   ```diff
   @@             Coverage Diff             @@
   ##           master   #36015       +/-   ##
   ===========================================
   + Coverage        0   48.48%   +48.48%     
   ===========================================
     Files           0      622      +622     
     Lines           0    45718    +45718     
     Branches        0     4975     +4975     
   ===========================================
   + Hits            0    22165    +22165     
   - Misses          0    22928    +22928     
   - Partials        0      625      +625     
   ```
   
   | [Flag](https://app.codecov.io/gh/apache/superset/pull/36015/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Coverage Δ | |
   |---|---|---|
   | [hive](https://app.codecov.io/gh/apache/superset/pull/36015/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `44.20% <ø> (?)` | |
   | [presto](https://app.codecov.io/gh/apache/superset/pull/36015/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `47.76% <ø> (?)` | |
   | [python](https://app.codecov.io/gh/apache/superset/pull/36015/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `48.47% <ø> (?)` | |
   | [unit](https://app.codecov.io/gh/apache/superset/pull/36015/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `100.00% <ø> (?)` | |
   
   Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#carryforward-flags-in-the-pull-request-comment) to find out more.
   </details>
   
   [:umbrella: View full report in Codecov by Sentry](https://app.codecov.io/gh/apache/superset/pull/36015?dropdown=coverage&src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).   
   :loudspeaker: Have feedback on the report? [Share it here](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
   <details><summary> :rocket: New features to boost your workflow: </summary>
   
   - :snowflake: [Test Analytics](https://docs.codecov.com/docs/test-analytics): Detect flaky tests, report on failures, and find test suite problems.
   - :package: [JS Bundle Analysis](https://docs.codecov.com/docs/javascript-bundle-analysis): Save yourself from yourself by tracking and limiting bundle sizes in JS merges.
   </details>


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org