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

Restructure rake test task runner #380

Merged
merged 3 commits into from
Mar 25, 2019

Conversation

sashadev-sky
Copy link
Member

@sashadev-sky sashadev-sky commented Mar 5, 2019

references #345, #355

  • tests pass -- rake test
  • code has been rebased on top of latest master (check if another pull request was added recently, and please rebase)
  • pull request is descriptively named and, if possible, multiple commits squashed if they're smaller changes

Thanks!

============
Updates made on top of mySQL5.7 PR updates (#355) I think that should be merged first anyway and then i'll just remove those from under this

Pending from chat with @jywarren in PR #355

  • In a comment above each one, explain the command to run each task
  • Include integration tests
  • Integrate with Travis

Notes on Ruby Warnings (unrelated to mySQL update)

  • Warning ::Fixnum is deprecated error: this warning will be fixed with an update to Rails 5. see here
  • File.exists? (deprecated since ruby 2.2.0) updated to File.exist? comes from gems and from the source code for running a rake test. Source code is updated to File.exist? for Rails 4. Current recommendation: turn off warnings
  • warning: loading in progress, circular require considered harmful also comes from gems. Current recommendation: turn off warnings.
  • warning: method redefined; discarding old <method_name>: also comes from gems and comes from any existing gem methods that are redefined in config/initializers. Updates to gems should start to fix some of these.
  • URI.escape obsolete error is in gems. Method is deprecated in favor of rail's to_param
  • warning: instance variable @____ not initialized: there are a bunch of these. to fix, we need to remove all of the instance variables from partials. I think maybe we should open a separate issue for this
    • also /app/app/views/layouts/_footer.html.erb:1: warning: assigned but unused variable - footer this error comes up for every partial with its filename as the respective unused variable. Not sure why put that into separate issue along with above
  • add lib/tasks/test_unit.rake file with rake task test runners for all functional and unit tests with the option to suppress warnings
    • Switching verbose to 'false' would create even less output but the tradeoff would be not being able to see the exact test file that is being run at each point in time
    • Under rake --tasks all tasks starting with the description "Autotask" are from this file

Problem causing gems
These gems throw the most warnings and the same warnings multiple times creating unreadable testing outputs. Maybe updates could fix this but it is also related to built in rails, active support, etc. settings so not sure. Would be good to open a separate issue for these.

  • paperclip seems to cause the greatest number of errors including errors mentioned above
  • openid (circular warnings)
  • will_paginate
  • friendly_id

============

@jywarren jywarren changed the title Rake test tasks Restructure rake test task runner Mar 5, 2019
@@ -0,0 +1 @@
/usr/local/mysql/lib/libmysqlclient.18.dylib
Copy link
Member

Choose a reason for hiding this comment

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

oh, is this required?

Copy link
Member Author

Choose a reason for hiding this comment

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

@jywarren macOS Mojave searches for this. It's configurations for mysql are a little different than any past system with macOS. It is not a file it's a sym link. Maybe @gauravano can double check that this doesn't effect any other users?

Copy link
Member Author

Choose a reason for hiding this comment

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

@jywarren sorry this is an issue for #355. Should I just remove all changes related to #355 out from under this? It was just a little easier for me to work with the same configs across branches but if it causes confusion it's not worth it.

Copy link
Member

Choose a reason for hiding this comment

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

Tried this PR locally, no issue due to this.

lib/tasks/test_unit.rake Outdated Show resolved Hide resolved
@jywarren
Copy link
Member

jywarren commented Mar 5, 2019

Ah i see you copied in everything here! Great! @siaw23 i wanted to ask your opinion on this way of running the tests, with maintainability and standardization in mind. Sasha's done some great work here, and it should help a lot, but is there anything we can do to ensure that it follows best practices and will be readable by most Rails devs? Just checking as I know you've worked on a lot of other Rails systems.

@jywarren jywarren requested a review from siaw23-retired March 5, 2019 16:45
README.md Outdated Show resolved Hide resolved
app/controllers/application_controller.rb Outdated Show resolved Hide resolved
@siaw23-retired
Copy link

Nice work btw @sashadev-sky!

@jywarren
Copy link
Member

jywarren commented Mar 5, 2019 via email

@sashadev-sky
Copy link
Member Author

@siaw23 Thanks for the advice! I updated the PR per your suggestions. Please note that #355 is the PR referencing mySQL, etc. upgrades. I branched this PR out from it to focus on rake test tasks. And that is what @jywarren was requesting additional input on. (See his comment above).

I still haven't integrated Travis will look into that next. Prior commits were for the mySQL branch.

@sashadev-sky
Copy link
Member Author

@jywarren Ok so I got this running with Travis and I think it is ready for merge.

If you reference the Travis build from my PR and a build from a recent PR such as this one yesterday, you will see that the testing output is the same but on mine most of the warnings are gone and readability is a lot better.

Another thing to note is that the current tests run so that a summary of each testing folder is outputted before moving on to the next, runs unit... says 13 tests passed... runs functional... says 25 tests passed, while mine just provides a summary at the end (38 tests passed). This is really the only difference and I can update that if you would prefer the former.

Please let me know your thoughts and how you would like to proceed! Thanks

@jywarren
Copy link
Member

Looks super. Thanks @sashadev-sky !!!

@jywarren jywarren merged commit 6ae5c23 into publiclab:main Mar 25, 2019
This was referenced Mar 26, 2019
avsingh999 pushed a commit to avsingh999/mapknitter that referenced this pull request Mar 30, 2019
* add a mysql setup file

* Squash commits
avsingh999 added a commit to avsingh999/mapknitter that referenced this pull request Mar 30, 2019
icarito pushed a commit that referenced this pull request Apr 7, 2019
* add a mysql setup file

* Squash commits
@sashadev-sky sashadev-sky deleted the raketesttasks branch June 13, 2019 04:54
chen-robert pushed a commit to chen-robert/mapknitter that referenced this pull request Dec 5, 2019
* add a mysql setup file

* Squash commits
jywarren added a commit that referenced this pull request May 5, 2020
* Shortening docker image in ~30%

* Caching bundle, gathering env variables and using newer sintax

* Creating startup script and env file

* Improving travis CI configuration

* Loading assets in production env

* Allow uglifier to interpret ES6

* Fix start command

* Fix travis script

* Tweak travis script

* Add delay

* Revert assets changes

* Return to Mysql5.7

* Tweak travis script

* Fix make redeploy-container command

* Add db migrate and precompile step.

* Add bower install to Makefile

* Clean after docker run. Avoid one bower run.

* Changes to be able to build container in Google Cloud

* Remove spurious symlink

* Copy config examples when making build

* Export env variable name

* Tag cloud image

* Add timeout

* Push to cloud registry

* Fix jenkins build error with docker-compose tty

* Add app to container and .dockerignore all else

* Copy configuration files when deploying to GCE

* Allow copy config to container

* Time extended (for cloud build & push)

* Delete redundant index.html.erb file (#427)

* Setupcoveralls (#438)

* Add coveralls

* Fix gemfile

* Fix env variable

* Add coveralls token

* Update README.md

* Remove legacy image controller code #404 (#417)

Deleted the lines from the selection indicated in the issue.

* Change comment count on comment creation via AJAX #441 (#443)

This closes issue #441 "Change comment count on comment creation via AJAX #441" by incrementing comments-number each time a new comment is added. This would ensure that the counter indicating the number of comments is increased without needing to refresh the page.

* update syntax of active record query(license method) (#439)

Fixes #437

* Docker improve rebased (#450)

* Shortening docker image in ~30%

* Caching bundle, gathering env variables and using newer sintax

* Creating startup script and env file

* Improving travis CI configuration

* Loading assets in production env

* Allow uglifier to interpret ES6

* Don't dettach when building container in travis

* Fix start command

* Fix travis script

* Try to resolve travis tests invocation

* Tweak travis script

* Add delay

* Bundle install before db setup

* Shortening docker image in ~30%

* Caching bundle, gathering env variables and using newer sintax

* Creating startup script and env file

* Improving travis CI configuration

* Loading assets in production env

* Allow uglifier to interpret ES6

* Fix start command

* Fix travis script

* Tweak travis script

* Add delay

* Revert assets changes

* Return to Mysql5.7

* Tweak travis script

* Fix make redeploy-container command

* Add db migrate and precompile step.

* Add bower install to Makefile

* Clean after docker run. Avoid one bower run.

* updte pr template (#448)

* Bump recaptcha from 4.13.1 to 4.13.2 (#452)

Bumps [recaptcha](https://github.com/ambethia/recaptcha) from 4.13.1 to 4.13.2.
- [Release notes](https://github.com/ambethia/recaptcha/releases)
- [Changelog](https://github.com/ambethia/recaptcha/blob/master/CHANGELOG.md)
- [Commits](ambethia/recaptcha@v4.13.1...v4.13.2)

Signed-off-by: dependabot[bot] <[email protected]>

* Restructure rake test task runner (#380)

* add a mysql setup file

* Squash commits

* Update README.md (#456)

* Change run to exec (#457)

* Bump paperclip from 4.2.4 to 4.3.7 (#285)

Bumps [paperclip](https://github.com/thoughtbot/paperclip) from 4.2.4 to 4.3.7.
- [Release notes](https://github.com/thoughtbot/paperclip/releases)
- [Changelog](https://github.com/thoughtbot/paperclip/blob/v4.3.7/NEWS)
- [Commits](thoughtbot/paperclip@v4.2.4...v4.3.7)

Signed-off-by: dependabot[bot] <[email protected]>

* Bump test-unit from 3.3.0 to 3.3.1 (#458)

Bumps [test-unit](https://github.com/test-unit/test-unit) from 3.3.0 to 3.3.1.
- [Release notes](https://github.com/test-unit/test-unit/releases)
- [Commits](test-unit/test-unit@3.3.0...3.3.1)

Signed-off-by: dependabot[bot] <[email protected]>

* Bump coveralls from 0.7.1 to 0.8.22 (#453)

Bumps [coveralls](https://coveralls.io) from 0.7.1 to 0.8.22.

Signed-off-by: dependabot[bot] <[email protected]>

* gridview aligned (#464)

* Alert improvement and adding byebug gem (#383)

* byebug gem added and alerts in separate file

* adding byebug history to gitignore

* adding timestamp to redirect

* added z-index to render login dropdown above leaflet icon

* fixed image partial rendering when no images (#423)

* fixed image partial rendering when no images.

* toggle no images <p> om upload

* fixed image partial rendering when no images.

* Bump recaptcha from 4.13.2 to 4.14.0 (#471)

Bumps [recaptcha](https://github.com/ambethia/recaptcha) from 4.13.2 to 4.14.0.
- [Release notes](https://github.com/ambethia/recaptcha/releases)
- [Changelog](https://github.com/ambethia/recaptcha/blob/master/CHANGELOG.md)
- [Commits](ambethia/recaptcha@v4.13.2...v4.14.0)

Signed-off-by: dependabot[bot] <[email protected]>

* add a flash error when adding tags and not logged in (#473)

* Upgrade app to Bootstrap 4 (#480)

* Bootstrap 4 small button fixes (#488)

* Add tests for comments and maps (#467)

* Updated query style (#436) (#469)

* Dynamic ports (#462)

* Dynamic port in compose file

* Omit setting container name

* Add initial sql dump entry

* Avoid resetting database on build

* Shortening docker image in ~30%

* Caching bundle, gathering env variables and using newer sintax

* Creating startup script and env file

* Improving travis CI configuration

* Loading assets in production env

* Tweak travis script

* Roll back to using Debian 9 with custom built GDAL (#489)

* Switch back to Debian 9 Stretch

* Simplify docker image

* Bump Ruby to 2.4.6

* Re-add dependency

* Add dependency (zip)

* Try pip install gdal

* Install libgdal-dev

* Revert attept to use pip

* Bump ruby

* Avoid naming containers in compose file

* Avoid overwriting database on redeploy-container

* Allow to load mysql dump

* Include own GDAL packages

* Disable ipv6 to prevent error

* Add missing Amazon S3 yml to Makefile

* Document unstable instance

* Changes to be able to build container in Google Cloud

* Copy config examples when making build

* Add app to container and .dockerignore all else

* Fixed missed merge

* Add db configurability by env vars for containers

* Fix db config

* Copy configs

* Switch keyserver

* Add env vars, tweak make

* Substitute env vars parameters

* Env var control

* Show env vars

* env not ENV

* add DB_SOCKET

* Add recomended parameters

* Not deploy app engine, show cloudsql dir

* Omit list cloudsql dir

* Added correct image tag

* Add database parameters as env vars

* Support $PORT env var

* Using Node 12 and Yarn for Dockerfile.txt as well

* Changing Passenger's port on production env

* Setting local db for travis

* set .env PORT to $PORT

* Remove .env

* Compose environment variableZ fallback

* Revert all files under /app to versions in main

* Revert to main

* Delete unneeded files

* Remove extra files from rebase

* Add bundle install as build step

* Deleted not needed Dockerfile

* Missed RUN in Dockerfile

* Add precompile step

* Hardcode environment at build time

* Adding missing yaml and update bootsnap version

* Omit /app/tmp from volume

* Revert try to get precompile to work

* Clean up patch for merging

* List variables in app.yaml

* Tweak for jenkins

* Add .env for jenkins

* Fix PORT for jenkins/docker-compose

* Address PORT properly

* New form ports

* Enclose docker-compose ports in quotes

* ports yaml should be object not array

* Try different format for ports

* Try docker-compose format

* Redirect script for AppEngine

* Try to revertt to working condition for appengine

* Point PORT in Procfile

* Revert to known good config in appengine

* Tweak assets precompilation

* Restore PORT setting

* Add redirect to map /warps directory to legacy archive

* Add .env for jenkins/docker-compose

* Add hardcoded route to legacy warps

* Remove .env for appengine

* Satisfy appengine docker-compose

* Ignore app.yaml

* Ignore app.yaml secrets

* Satisfy Jenkins wihout hurting appengine hopefully

Co-authored-by: Sebastian Silva <[email protected]>
Co-authored-by: rarrunategu1 <[email protected]>
Co-authored-by: Jeffrey Warren <[email protected]>
Co-authored-by: Milo MacPhail <[email protected]>
Co-authored-by: Sonali Agrawal <[email protected]>
Co-authored-by: Ananya Agrawal <[email protected]>
Co-authored-by: dependabot[bot] <dependabot[bot]@users.noreply.github.com>
Co-authored-by: Sasha Boginsky <[email protected]>
Co-authored-by: Kaustubh Nair <[email protected]>
Co-authored-by: Divya Baid <[email protected]>
Co-authored-by: Gaurav Sachdeva <[email protected]>
Co-authored-by: Govind Jeevan <[email protected]>
Co-authored-by: Cess <[email protected]>
Co-authored-by: Stefanni <[email protected]>
Co-authored-by: hc-barker <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants