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

Slider functional tests #322

Merged
merged 13 commits into from
Nov 8, 2017
Merged

Conversation

pottedmeat
Copy link
Contributor

Type: feature

The following has been addressed in the PR:

  • There is a related issue
  • All code matches the style guide
  • Unit or Functional tests are included in the PR

Description:

Adds basic functional tests for Dialog based on lzhoucs's PR #178 with rebasing and updates.

Resolves #158

@pottedmeat pottedmeat mentioned this pull request Oct 20, 2017
3 tasks
Copy link
Member

@bitpshr bitpshr left a comment

Choose a reason for hiding this comment

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

Travis looks like it's having some issues, but this looks good otherwise.

@dylans dylans added this to the 2017.10 milestone Oct 27, 2017
@kitsonk kitsonk removed this from the 2017.10 milestone Oct 30, 2017
Copy link
Contributor

@smhigley smhigley left a comment

Choose a reason for hiding this comment

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

I'm a little iffy on adding extraClasses just for easier targeting in tests, but I'm OK with it if you think it's worth it.

.findByCssSelector(`.${css.input}`)
.getProperty('value')
.then((value: string) => {
currentValue = parseInt(value, 10);
Copy link
Contributor

Choose a reason for hiding this comment

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

I think this works because all the example page sliders are from 0-100, but it's probably worth noting that if that ever changes, this check will fail.

})
.end();
}
}
Copy link
Contributor

Choose a reason for hiding this comment

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

One other potential test: clicking on the UI elements (or at least on the x,y position of the visible UI) sets focus on the <input>

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added in a bunch of tests for this

@pottedmeat pottedmeat force-pushed the slider-functional-tests branch from 9b6a78e to fd9d745 Compare October 31, 2017 18:03
@@ -65,6 +66,7 @@ export class App extends WidgetBase<WidgetProperties> {
}),
v('h1', {}, ['Disabled slider']),
w(Slider, {
extraClasses: { root: 's2' },
Copy link
Member

Choose a reason for hiding this comment

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

as with label PR, should wrap in a div with a functional test class then use that along with slidercss.root to target.

@codecov
Copy link

codecov bot commented Oct 31, 2017

Codecov Report

Merging #322 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #322   +/-   ##
=======================================
  Coverage   97.78%   97.78%           
=======================================
  Files          25       25           
  Lines        1940     1940           
  Branches      491      491           
=======================================
  Hits         1897     1897           
  Misses         20       20           
  Partials       23       23

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 9a5d126...db1ad29. Read the comment docs.

@pottedmeat pottedmeat force-pushed the slider-functional-tests branch 2 times, most recently from 140c574 to 9d6e627 Compare November 7, 2017 15:37
@pottedmeat pottedmeat force-pushed the slider-functional-tests branch from 9d6e627 to 8b211bd Compare November 8, 2017 15:24
@pottedmeat pottedmeat merged commit 9566d25 into dojo:master Nov 8, 2017
@pottedmeat pottedmeat deleted the slider-functional-tests branch November 8, 2017 22:16
@dylans dylans added this to the beta.4 milestone Dec 22, 2017
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.

7 participants