Rails upgrade, Ember.js upgrade, Bootstrap upgrade, and test suite unification #312
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…into admin-upgrade
I'm not exactly sure where this stands, but I'm pretty sure it was tied to this admin-upgrades branch, so rather than letting this sit in my local development environment, let's commit it.
The initial plan to upgrade things in more granular pieces didn't really
pan out, so this is unfortunately a "change everything" type of commit
(basically, there were too many hurdles in trying to upgrade only Ember
without touching other libraries like Bootstrap, since a variety of
those other libraries have better compatibility/ember addons available
with newer versions).
So...
- Upgrade to Ember 2.6 (up from our earlier 1.13 attempts in this
branch).
- Upgrade to Bootstrap 3.3.
- Upgrade most other library dependencies, remove some unused ones, and
replace most local vendor dependencies with bower dependencies.
- Get the basics of client-side authentication working with
ember-simple-auth with a bit of a hybrid solution so we can reuse the
existing server-side login page.
- Get basics of i18n integration working with the server side app.
- Come up with our own form-builder for generating the forms in a
bootstrap-friendly way. EasyForm isn't compatible with Ember 2, and
most of the current form builders out there are in flux or aren't
super-customizable to match our existing interface. But It turns out
with Ember 2 components, it actually isn't too hard to do this,
particularly since we can hard-code our assumptions rather than being
super-generic.
- Switch to Ember Data for models. ember-model doesn't work in Ember 2,
so we need to use something else. I also looked at ember-restless
which is more ember-model like, but figured I'd take the plunge with
ember-data in the hopes we'll have easier upgrade paths (since the
Ember folks maintain it). It's still not an awesome fit with some
of our custom-ish APIs and embedded records, but now that embedded
records are supported, we can make it work.
- Switch other ember libraries to versions that are compatible with
Ember 2 (like ember-cp-validations).
- Get the basics (index table and add/edit form) working again in the
following sections:
- API Users
- Admin Accounts
- API Scopes
- Admin Groups
- Website Backends
- For the areas we've updated to get working, we've basically
componitized everythin in the Ember 2 way (no more views or
controllers).
- Standardize on more ES6 styling of the code.
The primary remaining pieces to update are the API Backends section and
the Analytics pages. Those are also the biggest sections, but hopefully
this at least establishes the basic upgrade path for the other areas.
This is the ember2-ified version of our tables of child records in the API Backend forms which then open a modal form for editing. This is fortunately a lot less hackish in Ember 2. - Use ember-bootstrap for modal support (which in turn uses ember-wormhole to deal with all the funky DOM stuff surrounding modals). - Use ember-buffered-proxy for a nicer solution to handling the cancel/close event and throwing away changes. - More generally, use a component based approach, so the table listing is a component and the modal is a separate component.
Since ember-cli's development server is heavily dependent on filesystem events to detect changes, the best option currently seems to be rsync and rsync-auto with Vagrant. Polling is also an option on the NFS mount, but the performance isn't very good (leading to frustrating delays before you can see your changes). This isn't totally ideal (it requires running "vagrant rsync-auto" on the host machine), but it seems like the best option for now. And I looked into whether a Docker development environment would provide a better alternative, but they currently have similar (or worse) file system performance issues with the default setup. However, they are making improvements, so something to keep our eyes on: https://forums.docker.com/t/file-access-in-mounted-volumes-extremely-slow-cpu-bound/8076 This also sets up the necessary routing so ember-cli's live-reload functionality works.
This seems more descriptive and gives us an easier name to refer to this component by (to distinguish it from other things like the admin APIs).
A few of the appeals and reasons for exploring this: - Better curl libraries and support. Node's HTTP library isn't necessarily the best suited for certain integration tests. We already have to skip it and use Node curl libraries in certain tests, but the node curl library we were using doesn't work in newer versions of Node, and the only node curl library that does isn't compatible with CentOS 6. - Randomized test ordering. I've been wanting this for a while in mocha to help make our test suite more robust. - Parallel test runner. Minitest has a nice built-in parallel test runner, which could potentially speed things up quite a bit for some of our longer-running tests that are mostly waiting on HTTP requests (for example all the timeout tests or logging tests). So this a bit of a demo to see how rewriting our integration tests in Ruby would work and whether it's something we want to pursue further.
- Don't display the validation errors until the form field has been interacted with. This prevents the page from initially loading with validation errors displayed. - Fix validations in the modals where we use BufferedProxy. We need to apply the validation class to the BufferedProxy instance.
To reflect the changes made on master in 53d1ef8
- Standardize and improve the delete flow so the prompts include the record name, and there's a confirmation display after deleting succeeds (or an alert on deletion failure). - Change how the components handle redirects, by using the routing service, rather than having to pass a custom action around. - Fix a few edit pages where the extra record information (creator/last updated, etc) weren't showing up.
To handle random test failures when things aren't an exact match.
- Ensure the admin is always authenticated before loading the admin app. With the new Ember upgraded app if the admin had logged in, but then quit their browser and reopened, the app was previously thinking the admin was authenticated (so then various ajax calls would fail). We now validate the server-side session on application load each time. - Show a loading spinner and hide the admin navigation until the admin is logged in. This prevents the app from flashing up on screen before being redirected to the login page. - Add <noscript> handling, so any user without JavaScript enabled gets a message instructing them that javascript is required for the admin app. - Fix the "interval" buttons in the admin analytics (to switch graphs between hourly, daily, weekly, etc) not working. - Update admin-ui npm and bower dependencies to latest versions. - Fix stylesheets not loading properly on the login page when not in the development environment (the paths to our precompiled assets weren't correct). - Improve reliability of some tests.
A couple other things seem to still be contributing to intermittent test failures here. But by splitting these up into separate tests (rather than having multiple "visit" calls inside one test) and clearing the memroy cache between tests, that seems to fix the odd issues with Poltergeist. This now seems to run smoothly after lots of repeated runs (fingers crossed).
This is based on the Netscape log format, so it's more similar to our nginx logs. But this also logs further details to help in debugging (right now to debug intermittent test failures, but since this logs is mainly for debugging purposes on production too, the extra details seem relevant there too).
Since we fixed the API tests to not inherit from Capybara tests, the "body" method is undefined (as it should be).
We saw a random test failure in the CI related to these, and since it makes sense that these tests would be more sensitive to parallel tests, let's go ahead and disable parallelization on these specific tests.
Don't parallelize some of the tests that are more sensitive to chunked timing. Ensure other requests are made with unique URLs to prevent any overlap preventing concurrent requests.
For our test of IPs with only countries, but no city and region, our Singapore test IP now returns a city (I think this just changed in GeoIP's database released today). So switch to another IP that only returns a country code, but no city or region to test something similar (the new IP isn't actually a valid country code, it represent anonymous proxies, but it's the only IP I could find that returns data with only a country code, which is what we really want to test here).
This should fix the eslint task not loading unless you already had nodejs on your PATH.
We've seen some strange "IOError: closed stream" errors originating from the open3 library usage in our CI environment a few times now. They seem to stem from the Test::Proxy::Dns::TestCustomServer#test_ip_changes_without_dropped_connections test, where we have a background thread constantly making DNS changes, which requires a system call to reload unbound. Since open3 has some internal threading, I think something funky might be going on that's not quite thread-safe with this setup. So let's try using backticks to run the shell command and capture output instead.
This fixes the previous binary being used only working under Ubuntu systems.
We had a few tests that would break if they happened to be run on a computer in the Denver timezone. Fix those tests so they will work regardless of the host machine's timezone. Also add time zone randomization to our test suite to better ensure our tests aren't timezone dependent.
This was referenced Dec 8, 2016
This was referenced Dec 16, 2016
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This pull request includes several different things that regrettably got a bit tied together:
Given all these changes (and the rather large scope of files changed as a result of each one), this pull request unfortunately isn't super easy to review. But since all the major tasks are basically complete, and I believe things are looking pretty good, I wanted to go ahead and get this merged into master. I've probably missed some things, and we still need to do a bit more polishing and testing before our next package release, but by getting this big code dump into master it will at least let us get back to tackling more discrete tasks based on all these updates.
For a summary of the code changes involved:
src/api-umbrella/web-app: The web-app component is upgraded from Rails 3.2 to Rails 4.2. This by itself, wasn't a huge change, but we're now on a supported version of Rails which is a good thing. The bigger change was that all the Ember.js code has been stripped out of the Rails app as part of the Ember 2 upgrade. This simplifies the "web-app" Rails component so it's really just serving up the internal APIs and handling admin logins.src/api-umbrella/admin-ui: This is the new home for the upgraded Ember 2 admin application. Functionally, everything remains the same as our old admin Ember app, but it's now setup as a standalone Ember CLI app (which is the newer and better supported approach to Ember apps that didn't exist when we first started). There were more changes involved in this upgrade, since it involved reorganizing some things to better fit with Ember's "components" approach, ES6-ifying things, and cleaning up various things that we had sort of hacked around in the old version (like query params support that's now baked into Ember in a much cleaner fashion). As part of this, the admin has been upgraded from Bootstrap 2 to Bootstrap 3, so various things changed as part of that too.test: There's now a single test suite for API Umbrella, rather than 2 different test suites. This is arguably something that we didn't need to completely change wholesale as part of these upgrades, but the Ember upgrade sort of started the ball rolling (since we did have to revisit our browser based tests that were previously part of the web app, and we were beginning to hit compatibility issues with the old version of NodeJS our old test suite depended on). I may have then gotten a bit carried away in revamping our entire test suite, but I'm hoping the results are easier to deal with and test against. There's more context in Should we consolidate our test suites? #305, but I have found the new test suite easier to debug, and it's seeming far more stable about intermittent test failures than what were seeing previously (see Intermittent test failures #303, although this perhaps isn't 100% solved yet).So overall, these various upgrades are mostly about getting us on supported versions of our dependencies and cleaning some things up. So hopefully by tackling these upgrades, it better positions the code base for future updates and will help make maintenance easier.