Skip to content
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

Fix selenium selector for collections #16754

Merged

Conversation

davelopez
Copy link
Contributor

@davelopez davelopez commented Sep 28, 2023

Fixes

FAILED lib/galaxy_test/selenium/test_workflow_invocation_details.py::TestWorkflowInvocationDetails::test_job_details - selenium.common.exceptions.TimeoutException: Message: Timeout waiting on XPATH selector [//span[@class="content-title name"][text()="forward"]] to become clickable.

Many thanks to @ElectronicBlueberry for figuring out the issue!!

How to test the changes?

  • This is a refactoring of components with existing test coverage.

License

  • I agree to license these and all my past contributions to the core galaxy codebase under the MIT license.

Co-authored-by: Laila Los <44241786+ElectronicBlueberry@users.noreply.github.com>
@davelopez davelopez force-pushed the fix_selenium_test_nested_list_multi_view branch from 66bab28 to 1c29d33 Compare September 28, 2023 12:16
@davelopez davelopez changed the title Attempt to fix selenium test failures Fix selenium selector for collections Sep 28, 2023
@davelopez davelopez marked this pull request as ready for review September 28, 2023 12:19
@github-actions github-actions bot added this to the 23.2 milestone Sep 28, 2023
@davelopez
Copy link
Contributor Author

Argh... still did not fix the flakiness of lib/galaxy_test/selenium/test_history_multi_view.py::TestHistoryMultiView::test_list_list_display but it passed in the previous attempt 😭

@ElectronicBlueberry
Copy link
Member

Looking at the screenshot from the test failure it seems like the test navigates away from the multi-history view.

@davelopez
Copy link
Contributor Author

Hmm... I thought that it didn't even go inside the sub-collection, but could it actually be that it went inside and then out again? And why does it pass locally? Could the selector be the cause here too? Too many questions... 😮‍💨

@davelopez davelopez force-pushed the fix_selenium_test_nested_list_multi_view branch from 747f59d to 886e95b Compare September 28, 2023 15:54
@davelopez davelopez marked this pull request as draft September 28, 2023 16:14
@davelopez
Copy link
Contributor Author

lib/galaxy_test/selenium/test_history_multi_view.py::TestHistoryMultiView::test_list_list_display PASSED [100%]

This is starting to look more and more like a Heisenbug 🙃

@davelopez davelopez force-pushed the fix_selenium_test_nested_list_multi_view branch 4 times, most recently from 26b1858 to c1aa0d0 Compare September 29, 2023 12:00
@davelopez
Copy link
Contributor Author

davelopez commented Sep 29, 2023

Alright... after some more debugging, I still can't understand what is going on with lib/galaxy_test/selenium/test_history_multi_view.py::TestHistoryMultiView::test_list_list_display.

This is the information I gathered so far:

  • If you run this test locally it passes ✅ (including prefix, headless or no, no matter what)
  • If you run this test in CI disabling any other CI tests it passes ✅
  • if you run the entire test suite in CI the test always fails 💥 (you can see it failing in other PRs)
  • If you run all selenium tests in CI disabling all other CI tests it fails 💥
  • if you run some selenium tests in CI disabling all other CI tests it passes ✅

When it fails, for the look of the debugging screenshots, it seems after clicking on the sub-collection test0 it doesn't drill down into that sub-collection.

0-multi_history_list_list_before_click_hid_1-screenshot

I double-checked the selector but everything seems fine, it is in the DOM and obviously, it drills down fine locally or when you disable most of the other selenium tests... I don't know how other selenium tests can really affect this... 😞

Co-authored-by: Laila Los <44241786+ElectronicBlueberry@users.noreply.github.com>
@davelopez davelopez force-pushed the fix_selenium_test_nested_list_multi_view branch from c1aa0d0 to 4e20d95 Compare September 29, 2023 15:38
@mvdbeek
Copy link
Member

mvdbeek commented Oct 1, 2023

I can get it to fail locally with:

./run_tests.sh --skip-common-startup -selenium lib/galaxy_test/selenium -- --num-shards=3 --shard-id=0

Given this, I would assume there's an id conflict somewhere ... that's a tough one unless you can spot the mixup from the component itself.

@mvdbeek
Copy link
Member

mvdbeek commented Oct 1, 2023

OK, so for posterity: you can run the test locally against a configured database with GALAXY_TEST_DBURI='postgresql://postgres:postgres@localhost:5555/galaxy123?client_encoding=utf8. Abort once you see the test failure, find the failing job from the test log, find the test user through the job in the database. Configure your normal instance to use that database, register an admin account, impersonate the user that was active during the failing job. Then open the multi-history view, and indeed you can't navigate into the nested collection in question.

@davelopez
Copy link
Contributor Author

Thank you @mvdbeek! I will try out this technique and see if I can get to the bottom of this 👍

@davelopez
Copy link
Contributor Author

davelopez commented Oct 2, 2023

Ok, I tried to use the same DB for running the selenium test and then connect to the same DB in my local instance but for some reason, the user that is supposed to have that history doesn't have that history... 🤯
I'm sure I'm doing something wrong... but it is basically using the same DB connection URI for both the selenium run and the local instance if I understood it correctly...

SeleniumDebugging.mp4

Update

For some unknown (to me) reason, the history was marked as deleted for the impersonated user. That is why I couldn't see it in the video above. Once I undeleted it I can now fully reproduce the issue.

I need to do more debugging but the first thing I detected is that when you click on the collection it seems it does nothing... but it is drilling down the collection, the actual problem is that the collection ID seems to be the parent collection, which creates a loop... so the more you click on the collection the more you drill down in the stack to the "same" parent collection. Anyway... one step forward to solving this mystery.

@davelopez
Copy link
Contributor Author

davelopez commented Oct 3, 2023

Now everything is starting to make sense, but still, I have a piece of the puzzle missing in my head...

The "collection loop issue" is caused by this change I made in #16725

const collection = await collectionElementsStore.getCollection(itemObject.id);

I was wrongly assuming the ID of the DCObject (DatasetCollection) was the same ID used to query api/dataset_collections/{id} so I fetched the "collection" from the store to drill down.

These are the values of the variables in the case of the test failure:

itemObject: DCObject
{
    "id": "df7a1f0c02a5b08e", <-- This is a DatasetCollection ID
    "model_class": "DatasetCollection",
    "collection_type": "list",
    "populated": null,
    "element_count": null,
    "contents_url": "/api/dataset_collections/df7a1f0c02a5b08e/contents/df7a1f0c02a5b08e", <- ID coincidence...
    "elements": []
}
collection: HDCA
{
    "id": "df7a1f0c02a5b08e", <- HDCA ID
    "name": "list:list",
    "history_id": "ee3742ed6d8b4a0c",
    "hid": 5,
    "deleted": false,
    "visible": true,
    "type_id": null,
    "type": "collection",
    "create_time": "2023-10-02T13:25:26.649540",
    "update_time": "2023-10-02T13:25:26.649545",
    "url": "/api/histories/ee3742ed6d8b4a0c/contents/dataset_collections/df7a1f0c02a5b08e",
    "tags": [],
    "history_content_type": "dataset_collection",
    "model_class": "HistoryDatasetCollectionAssociation",
    "collection_type": "list:list",
    "populated_state": "ok",
    "populated_state_message": null,
    "element_count": 1,
    "job_source_id": null,
    "job_source_type": null,
    "job_state_summary": {
        "all_jobs": 0,
        "new": 0,
        "waiting": 0,
        "running": 0,
        "error": 0,
        "paused": 0,
        "skipped": 0,
        "deleted_new": 0,
        "resubmitted": 0,
        "queued": 0,
        "ok": 0,
        "failed": 0,
        "deleted": 0,
        "upload": 0
    },
    "contents_url": "/api/dataset_collections/df7a1f0c02a5b08e/contents/0a248a1f62a0cc04",
    "collection_id": "0a248a1f62a0cc04",
    "populated": true,
    "elements": [
        {
            "id": "1e8ab44153008be8",
            "model_class": "DatasetCollectionElement",
            "element_index": 0,
            "element_identifier": "test0",
            "element_type": "dataset_collection",
            "object": {
                "id": "df7a1f0c02a5b08e", <- Coincidence...
                "model_class": "DatasetCollection",
                "collection_type": "list",
                "populated": true,
                "element_count": 3,
                "contents_url": null,
                "elements": [
                    {
                        "id": "c9468fdb6dc5c5f1",
                        "model_class": "DatasetCollectionElement",
                        "element_index": 0,
                        "element_identifier": "data0",
                        "element_type": "hda",
                        "object": {
                            "id": "2d9035b3fc152403",
                            "model_class": "HistoryDatasetAssociation",
                            "state": "ok",
                            "hda_ldda": "hda",
                            "history_id": "ee3742ed6d8b4a0c",
                            "tags": [],
                            "create_time": "2023-10-02T13:25:26.537999",
                            "validated_state": "unknown",
                            "peek": "<table cellspacing=\"0\" cellpadding=\"3\"><tr><td>TestData123</td></tr></table>",
                            "metadata_dbkey": "?",
                            "deleted": false,
                            "name": "data0",
                            "file_size": 12,
                            "visible": false,
                            "purged": false,
                            "uuid": "80a8fa98-5f94-43d1-b77e-deaddc8042c5",
                            "misc_blurb": "1 line",
                            "misc_info": null,
                            "hid": 2,
                            "metadata_data_lines": 1,
                            "file_ext": "txt",
                            "update_time": "2023-10-02T13:25:26.538003",
                            "history_content_type": "dataset",
                            "data_type": "galaxy.datatypes.data.Text",
                            "validated_state_message": null,
                            "genome_build": "?"
                        }
                    },
                    {
                        "id": "2a56795cad3c7db3",
                        "model_class": "DatasetCollectionElement",
                        "element_index": 1,
                        "element_identifier": "data1",
                        "element_type": "hda",
                        "object": {
                            "id": "5a1cff6882ddb5b2",
                            "model_class": "HistoryDatasetAssociation",
                            "state": "ok",
                            "hda_ldda": "hda",
                            "history_id": "ee3742ed6d8b4a0c",
                            "tags": [],
                            "create_time": "2023-10-02T13:25:26.538006",
                            "validated_state": "unknown",
                            "peek": "<table cellspacing=\"0\" cellpadding=\"3\"><tr><td>TestData123</td></tr></table>",
                            "metadata_dbkey": "?",
                            "deleted": false,
                            "name": "data1",
                            "file_size": 12,
                            "visible": false,
                            "purged": false,
                            "uuid": "bc2e2adb-5153-4140-9011-cc94ee7a9763",
                            "misc_blurb": "1 line",
                            "misc_info": null,
                            "hid": 3,
                            "metadata_data_lines": 1,
                            "file_ext": "txt",
                            "update_time": "2023-10-02T13:25:26.538008",
                            "history_content_type": "dataset",
                            "data_type": "galaxy.datatypes.data.Text",
                            "validated_state_message": null,
                            "genome_build": "?"
                        }
                    },
                    {
                        "id": "4b187121143038ff",
                        "model_class": "DatasetCollectionElement",
                        "element_index": 2,
                        "element_identifier": "data2",
                        "element_type": "hda",
                        "object": {
                            "id": "d413a19dec13d11e",
                            "model_class": "HistoryDatasetAssociation",
                            "state": "ok",
                            "hda_ldda": "hda",
                            "history_id": "ee3742ed6d8b4a0c",
                            "tags": [],
                            "create_time": "2023-10-02T13:25:26.538011",
                            "validated_state": "unknown",
                            "peek": "<table cellspacing=\"0\" cellpadding=\"3\"><tr><td>TestData123</td></tr></table>",
                            "metadata_dbkey": "?",
                            "deleted": false,
                            "name": "data2",
                            "file_size": 12,
                            "visible": false,
                            "purged": false,
                            "uuid": "93f3361e-c18c-42d9-8575-f4e03176d688",
                            "misc_blurb": "1 line",
                            "misc_info": null,
                            "hid": 4,
                            "metadata_data_lines": 1,
                            "file_ext": "txt",
                            "update_time": "2023-10-02T13:25:26.538013",
                            "history_content_type": "dataset",
                            "data_type": "galaxy.datatypes.data.Text",
                            "validated_state_message": null,
                            "genome_build": "?"
                        }
                    }
                ]
            }
        }
    ],
    "elements_datatypes": [
        "txt"
    ]
}

The mixed ID df7a1f0c02a5b08e was causing the drill-down collection stack to push always the same HDCA, creating the loop. That could explain why it failed only when more selenium tests were run vs. just running TestHistoryMultiView::test_list_list_display in isolation. There was a higher chance of hitting this mixed ID issue.

The missing part of the puzzle is the actual solution 😅 I need to wrap my head around the different concepts related to collections, HDCA, DCE, DC... but maybe someone more versed in collection internals can help... The previous solution relied on querying the contents_url for drilling down, but I wanted to use the fetcher to get all the typing benefits and make use of the store for caching results.

Any ideas on how to do the correct query here?

@davelopez
Copy link
Contributor Author

I've updated this PR description to fix only one of the tests (that these changes address). The other one will be tracked in its own issue (#16785), as it is not a selenium problem but an actual bug. So taking this out of draft.

@davelopez davelopez marked this pull request as ready for review October 4, 2023 15:13
@dannon dannon merged commit 605d8ca into galaxyproject:dev Oct 4, 2023
27 of 28 checks passed
@davelopez davelopez deleted the fix_selenium_test_nested_list_multi_view branch October 4, 2023 18:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants