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

Fixes #1631 - Warns user and aborts tests if server isn't right or if it's not in test mode. #1743

Merged
merged 1 commit into from
Sep 7, 2017

Conversation

brizental
Copy link
Contributor

So, to check that the server is in test mode, I make a request to our API and check that it's returning fixture data. This took a little while longer, because I had to find a workaround to add external modules to the intern.js config file, since you can't add them as you would on the test suites, because it's not loaded as an AMD module. I don't feel that my solution looks very good, but it works and I asked in the intern Gitter chat and the guys said it was a fine way to do it.

Anyways, I also removed code to login with real data to GitHub. I know that @miketaylr was kind of against it, but I found out that we actually removed the code that did that in #1604 , so it didn't work anyways... I think we could just open a new issue for that if you guys think it's necessary to have the option to test without fixture data.

I also removed the visibleByClass method, because it wasn't used anywhere and Leadfoot has a similar method (findDisplayedByClassName).

Lastly, I noticed that when you start the server in non-test mode it runs grunt before starting the server, but not in test mode. So I fixed that.

Hope it's alright to add all this to the same PR. I didn't think it was necessary to open new issues for these things.

r? @miketaylr @zoepage

@zoepage
Copy link
Member

zoepage commented Sep 3, 2017

@brizental great work :) there is an issue somewhere mentioning the geckobug. Could you reference this in the commit? (Sorry, on my phone right now which makes it hard to search for it).

Thanks!

@brizental
Copy link
Contributor Author

@zoepage #1590

Open another terminal and window type:
\x1b[32m npm run start:test\x1b[0m
or
\x1b[32m source env/bin/activate && python run.py -t\x1b[0m

This comment was marked as abuse.

This comment was marked as abuse.

This comment was marked as abuse.

var request = http.get(url("/api/issues/100"), function(response) {
// If the headers are from non-fixture data,
// they will have the "etag" property.
if (response.headers.hasOwnProperty("etag")) {

This comment was marked as abuse.

This comment was marked as abuse.

// TODO: when https://github.com/mozilla/geckodriver/issues/308 is fixed,
// remove this ugliness.

function login(context) {

This comment was marked as abuse.

This comment was marked as abuse.

This comment was marked as abuse.

Open another terminal and window type:
\x1b[32m npm run start:test\x1b[0m
or
\x1b[32m source env/bin/activate && python run.py -t\x1b[0m

This comment was marked as abuse.

tests/intern.js Outdated
// The port on which the instrumenting proxy will listen
proxyPort: 9090,

// A fully qualified URL to the Intern proxy
proxyUrl: "http://127.0.0.1:9090/",
siteRoot: siteRoot,
tunnel: "SeleniumTunnel",
tunnelOptions: {

This comment was marked as abuse.

This comment was marked as abuse.

@brizental brizental force-pushed the issues/1631/1 branch 2 times, most recently from 46febce to 1c8279d Compare September 6, 2017 21:30
@brizental
Copy link
Contributor Author

@miketaylr I made the changes you asked for, Mike. Had to squash all commits together because I did something stupid while rebasing the first time, lol sorry.

@miketaylr
Copy link
Member

Had to squash all commits together because I did something stupid while rebasing the first time, lol sorry.

Heh no worries. Been there!

@miketaylr
Copy link
Member

You rock, Bea -- thanks!

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