junaiddshaukat commented on PR #59939:
URL: https://github.com/apache/airflow/pull/59939#issuecomment-3755087476

   Hi @vatsrahul1001, thanks for the detailed review! I've addressed all your 
feedback:
   
   ### Changes Made:
   
   1. **Coordinated with PR #59990** - Renamed `AssetsPage.ts` → 
`AssetDetailPage.ts` to clearly distinguish from the merged `AssetListPage.ts`. 
Also renamed the test describe to `"Asset Details Page"`.
   
   2. **Added `beforeAll` for data setup** - Added setup that triggers 
`asset_produces_1` DAG and polls for completion before tests run, ensuring 
asset data exists in CI.
   
   3. **Fixed fragile selectors**:
      - Replaced `label.locator("..")` with 
`label.locator("xpath=./parent::*")` + `.first()`
      - Replaced `.chakra-popover__body` with 
`getByRole("dialog").getByRole("link")`
   
   4. **Test naming** - Changed to `"verify asset details and dependencies"` 
(removed "should")
   
   5. **Removed unused code** - Removed the `assetsTable` locator
   
   6. **Refactored duplicate code** - Created `verifyStatSection()` helper 
method used by both `verifyProducingTasks()` and `verifyScheduledDags()`
   
   Tests pass locally on Chromium and WebKit. Ready for re-review! Please have 
a look and if need any furthur changes let me know!


-- 
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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to