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

feat(tests): add full e2e coverage for Tooltip component #4871

Merged

Conversation

dmitry-zhemchugov
Copy link
Contributor

PR Checklist

Before creating new PR, please take a look at checklist below to make sure that you've done everything that needs to be done before we can merge it.

  • read and followed the CONTRIBUTING.md guide.
  • built and tested the changes locally.
  • added/updated tests.
  • added/updated API documentation.
  • added/updated demos.

@ghost ghost added the needs review label Dec 3, 2018
Copy link
Contributor

@ludmilanesvitiy ludmilanesvitiy left a comment

Choose a reason for hiding this comment

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

  1. Need to move basic testing scope to integration/tooltip_page_spec.ts and full scope of tests to full/tooltip_page_spec.ts
  2. Need to move all logic to PO methods and reuse it in tests
  3. Need to cover each DEMO, according to rule: 1 use-case = 1 describe, 1 step = 1 it
  4. Need to make full coverage for all demos inside component

@codecov
Copy link

codecov bot commented Dec 3, 2018

Codecov Report

Merging #4871 into development will decrease coverage by 0.2%.
The diff coverage is n/a.

Impacted file tree graph

@@               Coverage Diff               @@
##           development    #4871      +/-   ##
===============================================
- Coverage        75.85%   75.65%   -0.21%     
===============================================
  Files              237      237              
  Lines             7336     7336              
  Branches          1464     1464              
===============================================
- Hits              5565     5550      -15     
- Misses            1369     1376       +7     
- Partials           402      410       +8
Impacted Files Coverage Δ
src/chronos/i18n/it.ts 71.42% <0%> (-28.58%) ⬇️
src/chronos/i18n/pl.ts 72.22% <0%> (-11.12%) ⬇️
src/chronos/i18n/sk.ts 82.97% <0%> (-8.52%) ⬇️
src/chronos/i18n/cs.ts 86.44% <0%> (-6.78%) ⬇️
src/chronos/i18n/uk.ts 78.04% <0%> (-2.44%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 1662554...7c5e5d2. Read the comment docs.

@codecov
Copy link

codecov bot commented Dec 3, 2018

Codecov Report

Merging #4871 into development will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@             Coverage Diff              @@
##           development    #4871   +/-   ##
============================================
  Coverage        74.78%   74.78%           
============================================
  Files              281      281           
  Lines             8547     8547           
  Branches          1631     1631           
============================================
  Hits              6392     6392           
  Misses            1705     1705           
  Partials           450      450

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 0e77f25...99c24a8. Read the comment docs.

@ghost ghost added needs review and removed needs fix labels Dec 20, 2018
@dmitry-zhemchugov dmitry-zhemchugov force-pushed the e2e-tests-tooltip branch 3 times, most recently from 3f35e15 to 3fe3dc9 Compare December 20, 2018 17:15
package-lock.json Outdated Show resolved Hide resolved
cypress/full/tooltip_page_spec.ts Outdated Show resolved Hide resolved

isTooltipAppearOnFocus(baseSelector: string) {
cy.get(`${baseSelector} ${this.togglerTooltip}`).focus();
this.isTooltipAppearsAndVisible(baseSelector);
Copy link
Contributor

Choose a reason for hiding this comment

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

isTooltipAppearOnFocus => make just method focusOnTooltip(baseSelector, additionalSelector?:string)


};

placementsOfTheTooltipContainer = {
Copy link
Contributor

Choose a reason for hiding this comment

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

tooltipPlacements

};

placementsOfTheTooltipContainer = {
placementTop: { tooltipContainerClass: 'tooltip in tooltip-top bs-tooltip-top top show', name: 'Tooltip on top' },
Copy link
Contributor

Choose a reason for hiding this comment

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

top:
right:
auto:
left:
bottom:

cy.get(`${baseSelector} ${this.togglerTooltip}`).focus();
this.isTooltipAppearsAndVisible(this.body);
}
isCustomTriggersClickAppearsAndVisible(baseSelector: string, buttonName: string, tiggerEvent: string, tooltipText: string) {
Copy link
Contributor

Choose a reason for hiding this comment

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

not descriptive name

cypress/support/tooltip.po.ts Outdated Show resolved Hide resolved
cypress/support/tooltip.po.ts Show resolved Hide resolved
cy.get(`${baseSelector}`).should('to.have.descendants', this.containerTooltip);
cy.get(`${this.containerTooltip}`).should('be.visible').contains(tooltipText);
cy.get(`${baseSelector}`).contains('Toggle').click();
cy.get(`${baseSelector}`).should('not.to.have.descendants', this.containerTooltip);
Copy link
Contributor

Choose a reason for hiding this comment

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

need rework according to the previous comment

}
clickOnDemoMenu(subMenu: string) {
cy.get('add-nav').contains('a', subMenu).click();
}
Copy link
Contributor

Choose a reason for hiding this comment

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

need to remove it and reuse method from base component

Copy link
Contributor

Choose a reason for hiding this comment

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

need to remove it and reuse method from base component

Copy link
Contributor

Choose a reason for hiding this comment

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

need to remove it and reuse method from base component

@ludmilanesvitiy ludmilanesvitiy changed the title tests(tooltip): added e2e test for tooltip feat(tests): add full e2e coverage for Tooltip component Dec 21, 2018
@ghost ghost added needs review and removed needs fix labels Dec 21, 2018
@dmitry-zhemchugov dmitry-zhemchugov force-pushed the e2e-tests-tooltip branch 5 times, most recently from 167cee9 to 8a118f9 Compare January 3, 2019 13:38

describe('Placement tooltip', () => {

const basic = tooltip.exampleDemosArr.placement;
Copy link
Contributor

Choose a reason for hiding this comment

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

const basic => const placementDemo


describe('Dismiss tooltip', () => {

const dismissPlacement = tooltip.exampleDemosArr.dismiss;
Copy link
Contributor

Choose a reason for hiding this comment

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

const dismissPlacement => const dismiss or const dismissDemo


describe('Dynamic Content', () => {

const dynamicContentPlacement = tooltip.exampleDemosArr.dynamicTooltip;
Copy link
Contributor

Choose a reason for hiding this comment

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

dynamicContent

const basic = tooltip.exampleDemosArr.placement;
const placement = tooltip.tooltipPlacements;

it('when user on hovering on trigger btn in the Placement exmpl - tooltip appears on the setted placement', () => {
Copy link
Contributor

Choose a reason for hiding this comment

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

when user hover on Trigger btn

const dynamicContentPlacement = tooltip.exampleDemosArr.dynamicTooltip;

it('when user clicks on the Dynamic Content tooltip btn, Tooltip appears', () => {
tooltip.focusOnTooltip(dynamicContentPlacement);
Copy link
Contributor

Choose a reason for hiding this comment

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

clickOnTooltip

dismiss: 'demo-tooltip-dismiss',
dynamicTooltip: 'demo-tooltip-dynamic',
customContentTemplate: 'demo-tooltip-custom-content',
dynamicHtmlButton: 'demo-tooltip-dynamic-html',
Copy link
Contributor

Choose a reason for hiding this comment

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

dynamicHtml

Copy link
Contributor

Choose a reason for hiding this comment

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

just dynamicHtml

dynamicTooltip: 'demo-tooltip-dynamic',
customContentTemplate: 'demo-tooltip-custom-content',
dynamicHtmlButton: 'demo-tooltip-dynamic-html',
defaultTooltip: 'demo-tooltip-container',
Copy link
Contributor

Choose a reason for hiding this comment

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

appendToBody

Copy link
Contributor

Choose a reason for hiding this comment

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

appendToBody

const configuringDefaultsTooltip = tooltip.exampleDemosArr.configuringDefaults;

it('when user clicks on the Configuring tooltip btn, Tooltip appears & dismiss after mouse leave', () => {
tooltip.focusOnTooltip(configuringDefaultsTooltip);
Copy link
Contributor

Choose a reason for hiding this comment

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

click

cy.get(basic).as('basicDemo').find(tooltip.togglerTooltip).focus();
cy.get('@basicDemo')
.should('to.have.descendants', tooltip.containerTooltip);
it('when the user on hovering on the trigger btn, tooltip appears and dismiss after mouse leave', () => {
Copy link
Contributor

Choose a reason for hiding this comment

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

...user hover on the trigger btn...

}
clickOnDemoMenu(subMenu: string) {
cy.get('add-nav').contains('a', subMenu).click();
}
Copy link
Contributor

Choose a reason for hiding this comment

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

need to remove it and reuse method from base component

@dmitry-zhemchugov dmitry-zhemchugov force-pushed the e2e-tests-tooltip branch 5 times, most recently from 51338c5 to 5f3a11b Compare January 15, 2019 11:28
describe('Configuring defaults', () => {
const configuringDefaults = tooltip.exampleDemosArr.configuringDefaults;

it('when user clicks on the Configuring tooltip btn, Tooltip appears & dismiss after mouse leave', () => {
Copy link
Contributor

Choose a reason for hiding this comment

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

& dismiss after mouse leave - looks like need to remove it

dismiss: 'demo-tooltip-dismiss',
dynamicTooltip: 'demo-tooltip-dynamic',
customContentTemplate: 'demo-tooltip-custom-content',
dynamicHtmlButton: 'demo-tooltip-dynamic-html',
Copy link
Contributor

Choose a reason for hiding this comment

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

just dynamicHtml

dynamicTooltip: 'demo-tooltip-dynamic',
customContentTemplate: 'demo-tooltip-custom-content',
dynamicHtmlButton: 'demo-tooltip-dynamic-html',
defaultTooltip: 'demo-tooltip-container',
Copy link
Contributor

Choose a reason for hiding this comment

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

appendToBody

}
clickOnDemoMenu(subMenu: string) {
cy.get('add-nav').contains('a', subMenu).click();
}
Copy link
Contributor

Choose a reason for hiding this comment

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

need to remove it and reuse method from base component

@valorkin valorkin added this to the 3.2.1 milestone Feb 5, 2019
@ghost ghost assigned ludmilanesvitiy Feb 5, 2019
@valorkin valorkin merged commit c0c5ee2 into valor-software:development Feb 6, 2019
@ghost ghost removed the ready for merge label Feb 6, 2019
@valorkin
Copy link
Member

valorkin commented Feb 6, 2019

@dmitry-zhemchugov use commit convention properly
it's tests(tooltip) not feat(tests)

@dmitry-zhemchugov
Copy link
Contributor Author

via Hubstaff
User: Dmitry Zhemchugov

Project: int_ngx-bootstrap (OSS) - https://app.hubstaff.com/projects/304822
Date Range: 01/16/19 - 01/16/19
Work session total: 0:00:10

Grand total: 0:00:10

leo6104 pushed a commit to leo6104/ngx-bootstrap that referenced this pull request Oct 10, 2019
IraErshova pushed a commit to IraErshova/ngx-bootstrap that referenced this pull request Jan 20, 2020
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.

3 participants