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

Adds two working tests for labels and a setup for the login #272 #666

Closed
wants to merge 36 commits into from

Conversation

karlcow
Copy link
Member

@karlcow karlcow commented Aug 6, 2015

  • setup login
  • check the gear is available
  • add contactready

@karlcow
Copy link
Member Author

karlcow commented Aug 6, 2015

I had initially added a test for removing the contactready label, but the slowness of communication with github. Makes it difficult to test. I will create another set of tests.

A note the way we currently test is not perfect because we tested that our UI is working well, not that the label has been sent to github. We should probably do a reload of the page and then see if the test has been setup (which is shouting for mockup ;) )

@karlcow karlcow changed the title Adds two working tests for labels and a setup for the login Adds two working tests for labels and a setup for the login #272 Aug 6, 2015
@karlcow karlcow mentioned this pull request Aug 6, 2015
7 tasks
return this.remote
.setFindTimeout(intern.config.wc.pageLoadTimeout)
.get(require.toUrl(url(100)))
.findByCssSelector('.js-login-link').click()

This comment was marked as abuse.

This comment was marked as abuse.

This comment was marked as abuse.

This comment was marked as abuse.

This comment was marked as abuse.

Mike Taylor added 2 commits August 6, 2015 11:32
Fixes webcompat#656. Add tests to prevent regression stage filter params.
Fixes webcompat#660. Move all label namespace handling to models.
@miketaylr
Copy link
Member

A note the way we currently test is not perfect because we tested that our UI is working well, not that the label has been sent to github.

You could test for the presence of a flash error message. We send those if the communication with GitHub fails: https://github.com/webcompat/webcompat.com/blob/master/webcompat/static/js/lib/models/issue.js#L131

But re-loading the page and checking for the label also seems good.

@karlcow
Copy link
Member Author

karlcow commented Aug 13, 2015

ok :)

PASS: main - labels - label gear is visible (246ms)
PASS: main - labels - label widget is opening on click (352ms)
PASS: main - labels - Label appears once selected (445ms)
PASS: main - labels - Label has been sent to GitHub (3349ms)
PASS: main - labels - Removes a label (5063ms)
0/5 tests failed
0/5 tests failed

@miketaylr apart of the setup with the login process, this is PR ready.

@karlcow karlcow self-assigned this Aug 13, 2015
@miketaylr
Copy link
Member

Awesome @karlcow. Can you squash your PR down to just a few commits? 36 is a lot. ^_^

@@ -37,7 +37,7 @@ define(['intern/lib/args'], function (args) {
functionalSuites: [ 'tests/functional' ],

// A regular expression matching URLs to files that should not be included in code coverage analysis
excludeInstrumentation: /./
excludeInstrumentation: /\./

This comment was marked as abuse.

This comment was marked as abuse.

@karlcow
Copy link
Member Author

karlcow commented Aug 13, 2015

@miketaylr will 6 or 7 commits be acceptable (one per test) 🔨 😈

@miketaylr
Copy link
Member

@karlcow haha yeah. I should have specified it would be nice to not merge back in the older commits (from myself).

@karlcow
Copy link
Member Author

karlcow commented Aug 13, 2015

@miketaylr ok :) thanks. I think I will destroy this one and recreate a new branch starting from the last version of master. And re-play the commits. thanks. It seems easier.

@karlcow
Copy link
Member Author

karlcow commented Aug 14, 2015

Closing this one in favor of #675

@karlcow karlcow closed this Aug 14, 2015
@karlcow karlcow deleted the 272/2 branch August 14, 2015 05:57
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.

3 participants