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

[DRAFT] Additional Tests for the Preprints index page #1931

Open
wants to merge 3 commits into
base: feature/search-improvements
Choose a base branch
from

Conversation

bp-cos
Copy link
Contributor

@bp-cos bp-cos commented Aug 15, 2023

Purpose

Add additional tests to the preprints index page

Summary of Changes

Add additional tests to the preprints index page

Screenshot(s)

N/A

Side Effects

Better deployment

QA Notes

N/A

@bp-cos bp-cos requested a review from brianjgeiger August 15, 2023 18:15
@bp-cos bp-cos changed the base branch from feature/search-improvements to develop August 15, 2023 19:32
@bp-cos bp-cos changed the base branch from develop to feature/search-improvements August 15, 2023 19:34
Copy link
Contributor

@brianjgeiger brianjgeiger left a comment

Choose a reason for hiding this comment

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

A couple of minor typos and a question.


// And the lead yield is verified
// eslint-disable-next-line max-len
assert.dom(this.element.querySelector('[data-test-lead-yield]')).hasText('This is the lead', 'The lead in yielded correctly');
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
assert.dom(this.element.querySelector('[data-test-lead-yield]')).hasText('This is the lead', 'The lead in yielded correctly');
assert.dom(this.element.querySelector('[data-test-lead-yield]')).hasText('This is the lead', 'The lead is yielded correctly');


// And the row yield is verified
// eslint-disable-next-line max-len
assert.dom(this.element.querySelector('[data-test-row-yield]')).hasText('This is the row', 'The lead in yielded correctly');
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
assert.dom(this.element.querySelector('[data-test-row-yield]')).hasText('This is the row', 'The lead in yielded correctly');
assert.dom(this.element.querySelector('[data-test-row-yield]')).hasText('This is the row', 'The row is yielded correctly');

@@ -23,7 +22,6 @@ export default class BrandedHeader extends Component<InputArgs> {
@requiredAction onSearch!: (value: string) => void;
@tracked showingHelp = false;

providerModel?: ProviderModel;
Copy link
Contributor

Choose a reason for hiding this comment

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

This doesn't seem related to adding new tests. This would have been a good addition to the "Changes" section of the PR, to let reviewers know that this is expected and what's going on here. Do you not have a providerModel any more?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants