Increase test coverage #467

kaustubh-nair · 2019-03-31T12:23:57Z

Part of #304

Make sure these boxes are checked before your pull request (PR) is ready to be reviewed and merged. Thanks!

PR is descriptively titled 📑 and links the original issue above 🔗
tests pass -- look for a green checkbox ✔️ a few minutes after opening your PR -- or run tests locally with rake test
code is in uniquely-named feature branch and has no merge conflicts 📁
screenshots/GIFs are attached 📎 in case of UI updation
ask @publiclab/reviewers for help, in a comment below

We're happy to help you get this ready -- don't be afraid to ask for help, and don't be discouraged if your tests fail at first!

If tests do fail, click on the red X to learn why by reading the logs.

Please be sure you've reviewed our contribution guidelines at https://publiclab.org/contributing-to-public-lab-software

Thanks!

kaustubh-nair · 2019-03-31T12:24:53Z

app/controllers/comments_controller.rb

    if logged_in? && current_user.can_edit?(@comment)
-      Comment.update(@comment.id, :body => params[:comment][:body])


This line was not updating the comment and the tests were failing. That's why I had to change it

kaustubh-nair · 2019-03-31T12:42:51Z

@jywarren I just noticed the repo_token for coveralls is not loading. I wonder why that's happening?
It is working for some prs like this #462
but not for some 🤔

kaustubh-nair · 2019-03-31T12:45:20Z

Looks like the token is loading for push builds but not for pr builds

kaustubh-nair · 2019-04-01T20:38:54Z

Added some more tests..

kaustubh-nair · 2019-04-01T20:53:45Z

Hey @jywarren , please review this!
Thanks!

jywarren · 2019-04-01T22:29:14Z

Hmm, i don't know -- and what's going on with Travis tests here? Are they stuck? They had some issues at Travis last week i think... could it be related?

jywarren · 2019-04-01T22:29:19Z

And... thank you!

kaustubh-nair · 2019-04-02T17:49:20Z

The problem is that travis does not expose environment variables for pull requests from external repositories. You can read about it here https://docs.travis-ci.com/user/pull-requests#pull-requests-and-security-restrictions
This is a really restricting constraint. I'm trying to find a good workaround for this.

kaustubh-nair · 2019-04-02T17:52:55Z

The simplest way would be the make the REPO_TOKEN env variable public. But someone else can use it to send test coverage reports to coveralls. So I guess we can eliminate this approach?

kaustubh-nair · 2019-04-02T18:00:15Z

But their security concern also makes sense.
If an external pr has access to the environment variable, they could just send some malicious code that could print the variable out. So giving external prs access to environment variables is essentially the same as making it public. We might as well put the environment variable in our repository then?
Thoughts? @jywarren

jywarren · 2019-04-02T18:25:39Z

Hmm, so how do we do this in plots2, where it is hooked up? https://github.com/publiclab/plots2/tree/master/.travis.yml Thank you for thinking this through!

…

On Tue, Apr 2, 2019 at 2:00 PM Kaustubh Nair ***@***.***> wrote: But their security concern also makes sense. If an external pr has access to the environment variable, they could just send some malicious code that could print the variable out. So giving external prs access to environment variables is essentially the same as making it public. We might as well put the environment variable in our repository then? Thoughts? @jywarren <https://github.com/jywarren> — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#467 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AABfJ56EbIl08G0HsirNtfFqm4c6O-9Lks5vc5qvgaJpZM4cUR-m> .

kaustubh-nair · 2019-04-02T18:33:27Z

Is it working for plots2?
I just checked some recent pr builds and it looks like travis isn't sending coverage report for those prs
Like for example
https://travis-ci.org/publiclab/plots2/builds/512970864#L3498
and https://travis-ci.org/publiclab/plots2/builds/514791356#L4206
It works for internal prs like this one though
https://travis-ci.org/publiclab/plots2/builds/511213184#L3486

kaustubh-nair · 2019-04-02T18:35:50Z

Coverage reports on coveralls are mostly for push builds.
Not for pull requests I think

kaustubh-nair · 2019-04-04T10:22:03Z

@jywarren If you think the tests are fine then should we merge this? I'll send more tests in subsequent prs.

kaustubh-nair · 2019-04-04T10:22:35Z

And what should be done about the travis issue?
Should I open a new issue to discuss it there?

jywarren · 2019-04-04T22:33:44Z

Yes, let's do! Thank you. And yes i'll merge this. Thanks so much! Heads up @cesswairimu who is also looking at improving testing (to coordinate!)

kaustubh-nair · 2019-04-05T14:24:15Z

Awesome, thanks
@cesswairimu Please mention which sections you are improving on(whenever you do) so that we don't end up working on the same thing. 😃

cesswairimu · 2019-04-05T15:12:00Z

Yeah great idea, I had implemented comments_controller tests before I saw your pr 🤦‍♀️, I am currently implementing tag, images controller tests and figuring out the sessions_controller tests will open a wip too. Thanks @kaustubh-nair

* 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] <support@dependabot.com> * 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] <support@dependabot.com> * 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] <support@dependabot.com> * 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] <support@dependabot.com> * 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] <support@dependabot.com> * 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 <sebastian@fuentelibre.org> Co-authored-by: rarrunategu1 <parker.rose@ymail.com> Co-authored-by: Jeffrey Warren <jeff@unterbahn.com> Co-authored-by: Milo MacPhail <40954168+milomacphail@users.noreply.github.com> Co-authored-by: Sonali Agrawal <sonali9696@gmail.com> Co-authored-by: Ananya Agrawal <33188930+ananya@users.noreply.github.com> Co-authored-by: dependabot[bot] <dependabot[bot]@users.noreply.github.com> Co-authored-by: Sasha Boginsky <41092741+sashadev-sky@users.noreply.github.com> Co-authored-by: Kaustubh Nair <kaustubh.nair@iiitb.org> Co-authored-by: Divya Baid <32747809+divyabaid16@users.noreply.github.com> Co-authored-by: Gaurav Sachdeva <sachdeva.gaurav1997@gmail.com> Co-authored-by: Govind Jeevan <govindjeevan7@gmail.com> Co-authored-by: Cess <cessmbuguar@gmail.com> Co-authored-by: Stefanni <stefannibrasil@gmail.com> Co-authored-by: hc-barker <hc-barker@users.noreply.github.com>

kaustubh-nair commented Mar 31, 2019

View reviewed changes

kaustubh-nair changed the title ~~Add tests for comments controller~~ Increase test coverage Apr 1, 2019

kaustubh-nair closed this Apr 2, 2019

kaustubh-nair reopened this Apr 2, 2019

sashadev-sky mentioned this pull request Apr 3, 2019

Upgrade to Rails 4 #482

Closed

5 tasks

Add tests for comments and maps

1f69835

kaustubh-nair force-pushed the test_coverage branch from ad8b2f7 to 1f69835 Compare April 4, 2019 10:25

jywarren merged commit c8eba6e into publiclab:main Apr 4, 2019

kaustubh-nair mentioned this pull request Apr 5, 2019

Environment variables not loading on travis for external PRs #491

Closed

icarito pushed a commit that referenced this pull request Apr 7, 2019

Add tests for comments and maps (#467)

5ec6984

jywarren mentioned this pull request Apr 16, 2019

Attempt to install codecov #543

Merged

chen-robert pushed a commit to chen-robert/mapknitter that referenced this pull request Dec 5, 2019

Add tests for comments and maps (publiclab#467)

2aea7b3

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Increase test coverage #467

Increase test coverage #467

kaustubh-nair commented Mar 31, 2019 •

edited

Loading

kaustubh-nair Mar 31, 2019

kaustubh-nair commented Mar 31, 2019

kaustubh-nair commented Mar 31, 2019

kaustubh-nair commented Apr 1, 2019

kaustubh-nair commented Apr 1, 2019

jywarren commented Apr 1, 2019

jywarren commented Apr 1, 2019

kaustubh-nair commented Apr 2, 2019

kaustubh-nair commented Apr 2, 2019

kaustubh-nair commented Apr 2, 2019

jywarren commented Apr 2, 2019 via email

kaustubh-nair commented Apr 2, 2019

kaustubh-nair commented Apr 2, 2019

kaustubh-nair commented Apr 4, 2019

kaustubh-nair commented Apr 4, 2019

jywarren commented Apr 4, 2019

kaustubh-nair commented Apr 5, 2019

cesswairimu commented Apr 5, 2019

		if logged_in? && current_user.can_edit?(@comment)
		Comment.update(@comment.id, :body => params[:comment][:body])

Increase test coverage #467

Increase test coverage #467

Conversation

kaustubh-nair commented Mar 31, 2019 • edited Loading

kaustubh-nair Mar 31, 2019

Choose a reason for hiding this comment

kaustubh-nair commented Mar 31, 2019

kaustubh-nair commented Mar 31, 2019

kaustubh-nair commented Apr 1, 2019

kaustubh-nair commented Apr 1, 2019

jywarren commented Apr 1, 2019

jywarren commented Apr 1, 2019

kaustubh-nair commented Apr 2, 2019

kaustubh-nair commented Apr 2, 2019

kaustubh-nair commented Apr 2, 2019

jywarren commented Apr 2, 2019 via email

kaustubh-nair commented Apr 2, 2019

kaustubh-nair commented Apr 2, 2019

kaustubh-nair commented Apr 4, 2019

kaustubh-nair commented Apr 4, 2019

jywarren commented Apr 4, 2019

kaustubh-nair commented Apr 5, 2019

cesswairimu commented Apr 5, 2019

kaustubh-nair commented Mar 31, 2019 •

edited

Loading