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

[#9480] Add snapshot test for instructor search page #9581

Merged
merged 5 commits into from Mar 31, 2019

Conversation

rrtheonlyone
Copy link
Contributor

Part of #9480

Added a few simple test cases for the instructor search page.

  1. No search results
  2. Have search results

@rrtheonlyone rrtheonlyone changed the title Add snapshot test for instructor search page [#9480] Add snapshot test for instructor search page Mar 19, 2019
@TEAMMATES TEAMMATES deleted a comment from teammates-bot Mar 19, 2019

exports[`InstructorSearchPageComponent should snap with a student table 1`] = `
<tm-instructor-search-page
fbSessionDataTables={[Function Array]}
Copy link
Contributor

Choose a reason for hiding this comment

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

You should populate this too for one of the test cases

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Interestingly this is not being used in the html template itself

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think that part is still under migration? @wkurniawan07

@rrtheonlyone rrtheonlyone added the s.ToReview The PR is waiting for review(s) label Mar 21, 2019
@rrtheonlyone
Copy link
Contributor Author

Hi @monmanuela / @AyushChatto , this page is still under migration. I have done a bare bones snapshot test for the currently available components. Do see if it is okay.

Copy link
Contributor

@monmanuela monmanuela left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Contributor

@AyushChatto AyushChatto left a comment

Choose a reason for hiding this comment

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

LGTM :)

});

it('should snap with a feedback session table', () => {

Copy link
Contributor

Choose a reason for hiding this comment

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

Just one qn - why is this one blank?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sry, this was a KIV. I have removed it!

Copy link
Member

@wkurniawan07 wkurniawan07 left a comment

Choose a reason for hiding this comment

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

LGTM

@wkurniawan07 wkurniawan07 added s.ToMerge The PR is approved by all reviewers including final reviewer; ready for merging and removed s.ToReview The PR is waiting for review(s) labels Mar 29, 2019
@rrtheonlyone rrtheonlyone merged commit 99a46ad into TEAMMATES:master Mar 31, 2019
@rrtheonlyone rrtheonlyone deleted the course-search-snapshot branch March 31, 2019 03:22
@wkurniawan07 wkurniawan07 added this to the V7.0.0-beta.0 milestone Apr 10, 2019
@wkurniawan07 wkurniawan07 added this to Ongoing in Front-end and RESTful back-end Migration via automation Apr 10, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
s.ToMerge The PR is approved by all reviewers including final reviewer; ready for merging
Projects
No open projects
Development

Successfully merging this pull request may close these issues.

None yet

4 participants