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

Enables pushed app before starting #752

Merged
merged 1 commit into from
Jun 3, 2016
Merged

Conversation

johnnyman727
Copy link
Contributor

@johnnyman727 johnnyman727 commented Jun 2, 2016

Fixes #707.

Travis will likely have issues with unrelated tests until #754 is merged.

@HipsterBrown
Copy link
Contributor

@johnnyman727 How can I smoke test this update? Should I follow the steps here?

Do you know why some of the controller.js tests are failing?

@johnnyman727
Copy link
Contributor Author

@HipsterBrown that would be a good smoke test if you have a windows machine. Otherwise, I would just make sure t2 run and t2 push work as expected with t2 init-generated assets.

The controller.js tests are failing because of the t2 update --list tests. Somehow our SSL certs on the build server became invalidated (maybe out of date?) so the http.gets are failing. I made mocks for the requestBuildList function in #292 to get that build working on Travis but it might have been better form to break the fix out into a separate PR. :/ Anyway, once that PR lands, this PR should be green too.

@HipsterBrown
Copy link
Contributor

Ok thanks. I'll give that a shot when I get home tonight.

It looks like #292 is nearly ready to be merged but if it still needs some time, it might be worth breaking out those mocks into a separate PR still so we can get this fix merged as well. That's up to you though.

@johnnyman727
Copy link
Contributor Author

@HipsterBrown Okay, you'e sufficiently persuaded me to stop being lazy. I broke the unit test fix out into #754.

@rwaldron
Copy link
Contributor

rwaldron commented Jun 2, 2016

As a matter of principle, this shouldn't land until the tests are green—we're all agreed on that, right?

@rwaldron
Copy link
Contributor

rwaldron commented Jun 2, 2016

I've just given appveyor a kick to re-run those tests.

@johnnyman727
Copy link
Contributor Author

@rwaldron how do you force AppVeyer to re-run the tests?

@HipsterBrown
Copy link
Contributor

All green tests means LGTM! 👍

@johnnyman727 johnnyman727 merged commit 0d235dc into master Jun 3, 2016
@johnnyman727 johnnyman727 deleted the jon-start-enable branch June 3, 2016 18:09
@rwaldron
Copy link
Contributor

rwaldron commented Jun 3, 2016

There's a "re-build" button :)

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