FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

chore(tests): Add unit tests for each component. by riavalon · Pull Request #138 · angular/material.angular.io · GitHub

This repository was archived by the owner on Jan 4, 2026. It is now read-only.
/ material.angular.io Public archive

chore(tests): Add unit tests for each component. - #138

Merged
jelbourn merged 1 commit into
angular:masterfrom
riavalon:chore-unit-tests
Apr 13, 2017
ConversationCommits (1)ChecksFiles changed
Merged

chore(tests): Add unit tests for each component.#138
jelbourn merged 1 commit into
angular:masterfrom
riavalon:chore-unit-tests

Conversation

riavalon commented Mar 30, 2017

Copy link
Copy Markdown
Contributor

No description provided.

riavalon requested a review from jelbourn March 30, 2017 17:51
This was referenced Mar 30, 2017
Merged
Open

jelbourn commented Mar 31, 2017

Copy link
Copy Markdown
Contributor

Looks like CI is failing on an scss issue

riavalon commented Mar 31, 2017 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

I believe the scss issue was due to it building with @angular/material set to beta 1 instead of 2. There is an issue with one of the tests not passing on the ci now, and I'm looking into it 👍

riavalon force-pushed the chore-unit-tests branch 4 times, most recently from 466fdf1 to 717a1f3 Compare April 4, 2017 14:47
jelbourn reviewed Apr 8, 2017
View reviewed changes

jelbourn left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Looks good, just a few tests that I think we can cut.

Comment thread package.json Outdated
"@angular/forms": "^2.4.6",
"@angular/http": "^2.4.6",
"@angular/material": "^2.0.0-beta.1",
"@angular/material": "^2.0.0-beta.2",

jelbourn Apr 8, 2017

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Can revert this line now that we're on beta.3

Comment thread src/app/pages/homepage/homepage.spec.ts Outdated
fixture.detectChanges();
}));

it('should render four rows of content on the homepage', () => {

jelbourn Apr 8, 2017

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

I don't think we need these homepage tests since it's all just static content.

Comment thread src/app/shared/footer/footer.spec.ts Outdated
.querySelector('.docs-footer-links a');
const href = link.getAttribute('href');
const text = link.textContent;
expect(~href.indexOf('angular.io')).toBeTruthy();

jelbourn Apr 8, 2017

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Should be able to do

expect(href).toContain('angular.io');

Comment thread src/app/shared/footer/footer.spec.ts Outdated
expect(text).toContain('Learn Angular');
});

it('should show an angular logo in the footer', () => {

jelbourn Apr 8, 2017

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

I don't think a test for the this is necessary

Comment thread src/app/shared/navbar/navbar.spec.ts Outdated
fixture.detectChanges();
}));

it('should have four main links', () => {

jelbourn Apr 8, 2017

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

I think this test also isn't needed

riavalon force-pushed the chore-unit-tests branch 6 times, most recently from b85ea7a to 616a64d Compare April 11, 2017 17:20
jelbourn approved these changes Apr 13, 2017
View reviewed changes

jelbourn left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

LGTM

jelbourn merged commit fd8d1ea into angular:master Apr 13, 2017
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Show hidden characters
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

1 more reviewer
jelbourn jelbourn approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees

jelbourn

Labels

cla: yes

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

Back | FazBrowse Home | New Git URL