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

Record presentation playwright #2519

Merged
merged 11 commits into from
Aug 21, 2024
Merged

Record presentation playwright #2519

merged 11 commits into from
Aug 21, 2024

Conversation

jrchudy
Copy link
Member

@jrchudy jrchudy commented Aug 14, 2024

Migrate record presentation spec and remove old protractor spec files.

Changes in this PR besides the test migration:

  • Change resolverImplicitCatalog to 100 to avoid colliding with a catalog number that GA will use each time the tests run
    • we have 4 parallel configurations that will use catalogs 1-4, this spec was run in the 1st catalog in protractor, so using resolverImplicitCatalog = 2 was fine before
  • modify testRecordMainSectionValues to test inline related table values when the custom mode shows a single link
    • also test custom mode with multiple values
    • required a change to RecordsetColValue type in recordset-utils.ts
  • modify testRelatedTablePresentation to check if the table is in tabular mode, if it isn't click the button so the following tests won't fail
  • delete downloaded files locally and in GA
    • doesn't necessarily matter in GA since the server is a fresh install and cleaned up each time
  • Test CSV item in export dropdown and use test.skip for BDBag tests in CI

@jrchudy jrchudy requested a review from RFSH August 14, 2024 00:14
@jrchudy jrchudy self-assigned this Aug 14, 2024
Copy link
Member

@RFSH RFSH left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was able to run the test cases and everything is working as expected. I really like that you moved the one-time-use function in record-helpers to be directly inside the spec. I always disliked how that function has a generic name but it was implemented for just this one page.

My comments are mostly about reusing the functions that we've already added.

P.S. Please update this branch with the latest master changes. It still has the alias creation step, so I had to manually remove that to be able to run the test cases.

@jrchudy jrchudy requested a review from RFSH August 20, 2024 21:28
@jrchudy
Copy link
Member Author

jrchudy commented Aug 20, 2024

@RFSH I updating this PR with the changes you asked for and what I mentioned in slack about testing BDBag in GA. I also updated the top comment to include details about each change that was out of the ordinary.

The major change since you last reviewed that I want to draw attention to is about testing the main record section values:

  • modify testRecordMainSectionValues to test inline related table values when the custom mode shows a single link
    • also test custom mode with multiple values
    • required a change to RecordsetColValue type in recordset-utils.ts

I did notice that the "export" tests I wrote are almost the same as the ones in the other spec but decided not to add a helper function for this since clickAndVerifyDownload is the bulk of it and the setup to get the elements is the "difference" in both places (even though the dropdown options are the same).

Although, now that I'm thinking about it more, that's like having testShareCitModal so I will make this change real quick in both places

@jrchudy jrchudy merged commit c8b990d into master Aug 21, 2024
1 check passed
@jrchudy jrchudy deleted the record-presentation-playwright branch August 21, 2024 20:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants