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

Try potential fix for jruby hangs #5858

Merged
merged 1 commit into from
Oct 3, 2019

Conversation

deivid-rodriguez
Copy link
Member

I want to try out this simplecov PR in case it helps with the jruby hangs.

The PR doesn't mention anything about jruby, but the sympthoms are very similar: it always hangs right before the last parallel process finishes, no matter whether it's the cucumber or the RSpec suite.

So, I guess we can try it.

@javierjulio
Copy link
Member

Sounds like a good idea, thanks!

@deivid-rodriguez
Copy link
Member Author

This looks very promising since I run CI 7 times and no hangs so far. I needed to add an extra fix to the jruby activerecord adapter.

This will hopefully fix parallel_tests hanging on jruby right before
completion.
@deivid-rodriguez
Copy link
Member Author

deivid-rodriguez commented Sep 16, 2019

Oh well. So:

  • Simplecov PR was merged and released.
  • Jruby PR I'm not sure it's the right way/place to fix the crash.

I reverted the jruby PR for now, and upgraded to simplecov 0.17.1.

And we're still getting a crash on jruby :(

In my opinion, a crash is better than hanging, and since it's happening in the same place as before (right before generating the last coverage report), maybe it was also happening before, but was being hidden by the hang.

So I'd say let's upgrade.

I'll restart the failing job now.

@deivid-rodriguez
Copy link
Member Author

@javierjulio Seems like we got the hang again in https://app.circleci.com/jobs/github/activeadmin/activeadmin/22913, let's merge this one and see how it goes.

@deivid-rodriguez
Copy link
Member Author

Merging to accelerate other PRs.

@deivid-rodriguez deivid-rodriguez merged commit 2520389 into master Oct 3, 2019
@deivid-rodriguez deivid-rodriguez deleted the try_potential_fix_for_jruby_hangs branch October 3, 2019 13:58
amilano added a commit to grafiti-io/activeadmin that referenced this pull request Oct 28, 2019
…epo (#1)

* Fix order of versions for diff of 2.1.0 release (activeadmin#5769)

Putting the newer version last ensures that we get the expected diff. Otherwise, we see additions and removals switched.

* Configure GitHub sponsors (activeadmin#5771)

To give more visibility to the different funding options.

* Update fr.yml (activeadmin#5748)

* Update pundit links to canonical repo

* Use bundler 2.0.2 in CI (activeadmin#5774)

* Fix typo in authorization adapter documentation

Co-Authored-By: Javier Julio <javierjulio@icloud.com>

* Add a minimal git history to generated test apps (activeadmin#5775)

It's very common to make experimental modifications to test
applications. This makes it very easy to go back to the freshly
generated application without having to regenerate it, by running `git
checkout .`, or to even keep track of the experimental changes being
done using git.

* Fix pundit policy retrieving on an edge case (activeadmin#5777)

When retrieving the policy for an `ActiveAdmin::Page`, and the pundit
namespace is `:active_admin`, we need an extra check so that the right
policy is properly picked up.

* Add Macedonian (mk) locale (activeadmin#5710)

* Drop unnecessary stuff (activeadmin#5780)

We no longer need this, since we support rubies above or equal to 2.4.

* Fix indent of permitted parameters boilerplate code

* Symmetric spacing

The blank line before class end is hard to eliminate without making the
template messy. So I add a blank line before class begin so that the
generated class is a bit prettier.

* Use a consistent style for `permit_params`

* Remove unneeded new lines

* Move sample app page to its own file

* Move sample app resources to their own page

* Move test models to their own files

* Generate migrations instead of full models

The model generator only generates:

* Model, which we are fully overwriting later.
* Unit tests, which we don't use.
* Migration.

So using the migration generator seems better.

* Add a binstub for rubocop

* Enable `Layout/SpaceAroundOperators` cop

* Fix boilerplate code indentation

Broken when the template was reindented.

* List candidate fields for `permit_params`

Co-authored-by: coezbek <christopher@oezbek.org>

* Add some extra docs

Co-authored-by: coezbek <christopher@oezbek.org>

* Also use the candidate list in alternative syntax

* Relax dependency on railties (activeadmin#5786)

This gets us ready to support the final release of Rails 6.0, and also
allows activeadmin to be tried against Rails master more easily.

* Re-use I18n translations of formtastic ~ Labels (activeadmin#5764)

* Re-use I18n translations of formtastic ~ Labels
* Undo unnecessary change and add a spec
* Simplify

* Fix migration generator against Rails master (activeadmin#5788)

Current Rails master (6.1.0.alpha) adds a new --timestamps option to the
migration generator that it's true by default. So to get migrations
running under Rails master, we need to make sure we don't generate
duplicate timestamp columns by using both explicit columns and the new
(default) option at the same time.

* Skip error when running `bin/rake local` (activeadmin#5789)

Normally, you would run `bin/rake local server`, `bin/rake local
console`, or any other Rails command as the second argument. That
creates a test application with seed data if it doesn't already exist,
and then runs the specified Rails command on it.

However, if you just run `bin/rake local`, it will generate the
application but then fail with the error `bundler: exec needs a command
to run`. Instead of doing that, only generate the test application and
exit without error.

* Persist the correct Gemfile to each test app (activeadmin#5790)

With this, we no longer need to mess with `BUNDLE_GEMFILE`'s when
booting applications for different Rails versions, and each application
will always use the right Gemfile. We can even boot applications from
the root folder like this: `tmp/rails/rails-6.0.0.rc1/bin/rails s` and
it will just work.

* Use double quotes for consistency

All other locales are using double quotes for the nested keys in the status_tag block except for this one.

* Separate status_tag `nil` case for optional locale customization

When using the `status_tag` component with boolean fields, both `nil` and `false` were treated the same and had the same output but without a way to modify that other than overriding the component.

These 2 conditions are now split, so for the `nil` case it returns "Unset" but the locale will keep a display value of "No" for that locale key. That **will maintain backwards compatibility** so when `status_tag` is called with `nil` or `false` it will continue to display the same "No" label but provide the option to customize the label for the `nil` case only, leaving the `false` one as is.

Further, we need to ensure the same CSS class for presentation due to the change, so the class list output now includes "no" and "unset".

* Update all locales with status_tag "unset" key

For each locale, the value of "unset" is just defaulted to the same value from the "no" key to preserve existing behavior but enabling a user to customize the display label.

* bundle update yard (activeadmin#5798)

* Update status_tag docs regarding boolean values

* Add changelog for separate status_tag nil/false labels

* Enable `Style/ParallelAssignment` cop (activeadmin#5803)

* Make sure show page title is escaped (activeadmin#5802)

* Extract an `app_name` method in the generator

* Extract a `gemfile_from_env` method

* Base test app names off the current Gemfile

* Pass gemfile full path to Rails generator

* Keep instantiated generator

So we can ask any information about the generated application that we
need.

* Rename test application class

Since it's used for more than generating the application, but also for
asking information about it.

* Ask expanded gemfile to test application

* Ask full path to the test application class

* Track coverage for test app creation too

* bin/rake bundle[update,simplecov]

* Add a way to forcefully create the test application

I find myself running `rm -rf tmp/rails/rails_52` or whatever and then
running `bin/rake setup` to recreate the test application. Now I can do
`bin/rake setup:force`.

* Consistency rename

* Move test-report persistance to separate task

Otherwise we will accidentally restore coverage files created by the
test app creation jobs, and that will cause conflicts.

* Revert "Ensure application gets reloaded only once" (activeadmin#5801)

This reverts commit 21b6138, because it
created a performance regression in development when reloading code.

* Use non vulnerable `lodash` for `gherkin-lint` (activeadmin#5806)

* Get ready for 2.2.0 release (activeadmin#5805)

* Bump Rails to latest 6.0.0.rc2

Rails recently released version 6.0.0.rc2 https://github.com/rails/rails/releases/tag/v6.0.0.rc2

* fix csv Builder ignoring global humanize_name setting

* add features testing global csv humanize_name setting

* add changelog entry

* Use the ransack master version because is compatible with Rails 6.0.0.rc2

* Remove unnecessary condition

* Extract an utility method

* Remove blank line for consistency

* Fix crash when filtering by nested resource

* Make posts a nested resource (activeadmin#5814)

So we have an example of a nested resource in our test application.

* Don't abort `bin/rake` when test application exists (activeadmin#5813)

* Remove uneeded skips (activeadmin#5818)

* Another test app generation fix (activeadmin#5819)

* Fix `belongs_to` specification (activeadmin#5817)

I needed to change to association name to make filtering by post in the
user's index page, otherwise, the applied filter cannot be properly
displayed.

But then I need to make sure routes and parameters still use "user".

* Hide "Last" link when `pagination_total` is false (activeadmin#5822)

* Update documentation for including associated objects (activeadmin#5824)

* No need to depend on specific plugins

`github-pages` already depends on the most usual plugins.

* Make sure the default layout is applied to pages

Otherwise we get unstyled pages locally.

* Make sure markdown relative links are handled

Otherwise we get broken links to non converted .md pages in development.

* Uncomment commented out step

* Rename feature to better match what it tests

* Remove unneeded blank line

* Support optional belongs_to with renamed resources

Co-authored-by: Kris Dekeyser <kris.dekeyser@libis.be>
Co-authored-by: David Rodríguez <deivid.rodriguez@riseup.net>
Co-authored-by: Nathan Broadbent <git@ndbroadbent.com>

* Fix a few more issues

By simplifying the setup to not use namespaces but use just renames of
the actual classes, two more issues reproduced.

* Not being able to infer the correct parent class to retrieve records
  from:

  ```
    And I click "Author Articles"                                 # features/step_definitions/index_views_steps.rb:1
      undefined method `find' for nil:NilClass (NoMethodError)
      ./lib/active_admin/resource_controller/data_access.rb:74:in `scoped_collection'
      ./lib/active_admin/resource_controller/data_access.rb:57:in `find_collection'
      ./lib/active_admin/resource_controller/data_access.rb:44:in `collection'
      ./lib/active_admin/resource_controller/streaming.rb:12:in `index'
      ./features/step_definitions/index_views_steps.rb:2:in `/^I click "(.*?)"$/'
      features/renamed_resource.feature:66:in `And I click "Author Articles"'
  ```

  Fixed by passing the correctly renamed class to inherited_resources's
  `belongs_to` helper.

* Parent class was not being properly "translated" before calling the
  polymorphic route helpers:

  ```
    And I click "Author Articles"                                 # features/step_definitions/index_views_steps.rb:1
      undefined method `new_admin_user_article_path' for #<Admin::ArticlesController:0x000055b6ee43c7b8>
      Did you mean?  new_admin_article_path
                     new_admin_user_password_path
                     new_admin_author_article_path
                     new_admin_user_session_path (ActionView::Template::Error)
      ./lib/active_admin/resource_controller/polymorphic_routes.rb:12:in `polymorphic_path'
  ```

Fixed that it by looking at the parent renamed resource to do the
mapping so that the polymorphic helpers generate the correct route.

* Point to devise master

Since latest release is not yet Rails 6 final compatible.

Changeset generated with `bundle && bin/rake bundle[update,devise]`
after pointing devise to master.

* Use the latest ransack release

That should be compatible with all supported Rails versions.

Changeset generated through `bin/rake bundle[update,ransack]` after
removing the pin to ransack's master.

* Test against Rails 6 final

Changeset generated through
`bin/rake bundle[update,rails,inherited_resources,rails-i18n]`.

* Add some spacing in HTML for readability

* Remove unnecessary spacing from snippet

* Explain the Tidelift subscription in the homepage

* Bump minimum ransack requirement to 2.3 (activeadmin#5831)

* Bump nokogiri from 1.8.5 to 1.10.4 in /docs (activeadmin#5833)

Bumps [nokogiri](https://github.com/sparklemotion/nokogiri) from 1.8.5 to 1.10.4.
- [Release notes](https://github.com/sparklemotion/nokogiri/releases)
- [Changelog](https://github.com/sparklemotion/nokogiri/blob/master/CHANGELOG.md)
- [Commits](sparklemotion/nokogiri@v1.8.5...v1.10.4)

Signed-off-by: dependabot[bot] <support@github.com>

* Update banner text and link

* Jruby 9.2.8.0 has been released (activeadmin#5834)

Maybe it will perform better and we won't have to restart jruby builds
anymore :)

* update method name in documentation (activeadmin#5836)

resource -> find_resource

* Add a "For enterprise" section in README

* Use a specific link for  the support section

To find out which kind of language resonates more.

* Switch to apparition capybara driver (activeadmin#5840)

To remove the chromedriver requirement.

* Test against Rails 6 final

* Use Rails 6 by default

* Use released devise officially supporting Rails 6

* bin/rake bundle[update,rspec-rails]

* bin/rake bundle[update,sqlite3]

* Rails 5.0 still needs sqlite3 1.3

Make that explicit.

* bin/rake bundle[update,cancancan]

* bin/rake bundle[update,pry-byebug]

* bin/rake bundle[update,cucumber-rails]

* bin/rake bundle[update,parallel_tests]

* bin/rake bundle[update,pundit]

* bin/rake bundle[lock,--update=activerecord-jdbcsqlite3-adapter]

* bin/rake bundle[lock,--update=jruby-openssl]

* Use the standard cucumber formatter everywhere (activeadmin#5844)

The "pretty" formatter could be helpful when debugging order dependent
failures (maybe), but other than that, it's just too verbose (specially
with parallel_tests), and it makes it hard to see the actual scenarios
that failed and the error messages.

* Make jruby hangs appear earlier (activeadmin#5847)

The `parallel_tests` gem has this feature where it will output a dot
every 60 seconds to avoid CIs that fail when no new output has not been
displayed for a while.

Our jruby build is hanging randomly and when that happens, this feature
makes it take hours for the job to fail.

In this commit, I'm essentially disabling this feature, by giving the
heartbeat a high value, so that we actually get these failures earlier.

* Normalize test app paths (activeadmin#5843)

* Fix bundler cache path

Since we run tests with the root user, CircleCI has been saving the
bundler cache to the wrong place.

* Remove unnecessary verbose flag

Too verbose :)

* Fully regenerate lock files

Just to trigger caching changes, and see if the fix works.

* Disable parallel_tests CI dots for RSpec too (activeadmin#5848)

The jruby hang is happening there too.

* Use rubygems 3.0.6 in CI

* Bump MRI 2.4 patch level in CI to 2.4.7

* Bump MRI 2.5 patch level in CI to 2.5.6

* Reword step to read better

* Use `When` instead of `Given` in a couple places

Because it reads better and because the step is defined using that
keyword.

* Move helper method to its only usage

It makes `selector_for` easier to understand.

* Generate a sample application using turbolinks

* Detach previous batch_action click handler before attaching new one

* Detach everything before re-attaching clicks and confirm handlers

* Add turbolinks entries to CircleCI

* Changelog entry for the turbolinks fix

* Workaround turbolinks flaky failure

Apparently, when loading the posts index page, the "View" link is
available without turbolinks behaviour attached for some time. I can
even reproduce this on the test application by navigating to an index
page, reloading the page, and quickly clicking on a "View" link: nothing
happens.

We would need to investigate this issue more deeply, but for now I'm
changing the feature to navigate directly to the page where the tabs
are, so that it doesn't hit this issue.

* Remove only the events that ActiveAdmin uses

Co-Authored-By: Javier Julio <JJfutbol@gmail.com>

* Remove spurious comment on a locale file (activeadmin#5851)

* Get ready for 2.3.0 release (activeadmin#5853)

* Revert "Bump minimum ransack requirement to 2.3" (activeadmin#5854)

This reverts commit d9d70c4, because
ransack 2.3 has an outstanding bug that affects our supported Rails, and
quite a lot of users.

* Get ready for 2.3.1 release (activeadmin#5856)

* Extract some common test logic for later reuse

* Make count query optimisation work when using decorate_with

* Remove now unneeded workaround (activeadmin#5859)

* Completize missing Slovak translations (activeadmin#5861)

* Make sure all PR links in changelog are implicit

* Sort contributors in changelog

* Avoid rendering huge associations (activeadmin#5548)

Render a text filter instead of a select for large associations (opt-in)

* Bump tested 2.6 ruby patch level to 2.6.5

* Bump tested 2.5 ruby patch level to 2.5.7

* Bump tested 2.5 ruby patch level to 2.4.8

* Get ready for 2.4.0 release (activeadmin#5872)

* Remove redundant \_ (activeadmin#5873)

* Improved german translations (activeadmin#5874)

Added translations that were in en.yml but not in de.yml

* Bump rubyzip from 1.2.2 to 2.0.0 in /docs (activeadmin#5875)

Bumps [rubyzip](https://github.com/rubyzip/rubyzip) from 1.2.2 to 2.0.0.
- [Release notes](https://github.com/rubyzip/rubyzip/releases)
- [Changelog](https://github.com/rubyzip/rubyzip/blob/master/Changelog.md)
- [Commits](rubyzip/rubyzip@v1.2.2...v2.0.0)

Signed-off-by: dependabot[bot] <support@github.com>

* Bump simplecov to 0.17.1 (activeadmin#5858)

This will hopefully fix parallel_tests hanging on jruby right before
completion.

* Document last minute release improvement (activeadmin#5876)

* Remove unneeded gitignore entry

I don't think our development environment generates such folder on a non
already gitignored location.

* Remove another unnecessary gitignore entry

I'm not sure how Rails handles this file, but we use the
`BUNDLE_GEMFILE` env variable which indirectly controls the Rails
version.

* Remove unnecessary gitignore entry

It's version manager specific, so it should live on each developer's
gitignore since there's nothinhg in this repo specific to a particular
ruby version manager.

* Tighten some gitignore entries

These folders are expected to be found on a single location at the root
of the repo. Only gitignore those.

* Lock sprockets in bug report template

Because it's not yet working with sprockets 4.

* Bump sassc-rails in bug report template

* Bump rails version in bug report template

* Remove redundant configuration

* Move less informative assertion to the end

If you can see the response body, you can figure out faster what's
wrong.

* Gitignore bug report template tmp folder

* Drop Rails 5.0 and Rails 5.1

Since they are no longer maintained by the Rails team.

* Centralize sqlite3 dev dependency again

Since it's now the same requirement for all supported Rails versions.

* Bump rubocop to 0.75.0 (activeadmin#5884)

I needed to remove

```yaml
Rails:
  Enabled: true
```

from the configuration, because whereas before (when Rails cops lived in
the core repo) it seemed to mean "Allow manually enabling specific Rails
cops", now (when Rails cops live as a separate extension) it apparently
means "Enable every Rails cop".

* Normalize link references in CONTRIBUNTING file

Consistently use shortcut notation.

* Remove unneeded stuff

We commit our lockfiles, so this shouldn't happen.

* Remove unintended numbering

* Add a section explaining the changelog format

* Add a pull request template with PR instructions

* Remove duplicated sentence

The bug report template starts with this exact text and link. I don't
think we need to repeat it again.

* Fix incorrect anchor

* Use filter label when condition has a predicate (activeadmin#5886)

* Move javascript sources

* Change es6 extension to js

* Rename 'src' to 'active_admin' within javascript

* Add rollup config

* Add bundled js

* Replace window.activeadmin usage with es6 import

* Remove sprockets-es6 as a dependency

* Delete package-lock.json

* Add javascript bundle checking to specs

* Add auto-generated warning and blank line in bundled js

* Update packaging to match current folder structure and expose js and scss

* Add yarn binstub

* Update documentation to explicit yarn/rollup/node requirements

* Fix error when routing with array containing symbol. (activeadmin#5870)

* Rename js files replacing underscore with hyphens

* Move active-admin.js to utils.js

* Invert js import logic

* Split initializers and classes

* Update base imports to use only initializers

* Give each scenario only the setup it needs

* Fix conflict between meta tags and `Tag` models

* Remove Gemfile.common

* Use existing gemfile in docs

* Allow specifying custom input_html for DateRangeInput (activeadmin#5867)

closes activeadmin#5864

* Move each Gemfile to its own folder

* Bump bundler version in docs Gemfile

* Add dependabot config

* Rename some steps

To distinguish them from other caching.

* More explicit grouping to hint dependabot

* Update `rake` to 13.0.0

It's handy to do this now to force the cache to be regenerated at the
right place.

* Bump rspec-rails from 3.8.2 to 3.9.0 in /gemfiles/rails_52 (activeadmin#5898)

* Bump db-query-matchers from 0.9.0 to 0.10.0 in /gemfiles/rails_52 (activeadmin#5904)

* Bump rspec-rails from 3.8.2 to 3.9.0 (activeadmin#5900)

* Bump mdl from 0.5.0 to 0.6.0 (activeadmin#5906)

* Bump db-query-matchers in /gemfiles/rails_60_turbolinks (activeadmin#5899)

* Bump db-query-matchers from 0.9.0 to 0.10.0 (activeadmin#5913)

* Bump rspec-rails from 3.8.2 to 3.9.0 in /gemfiles/rails_60_turbolinks (activeadmin#5912)

* Bump jasmine-core from 2.9.1 to 2.99.2 (activeadmin#5911)

* Bump jasmine-core from 2.9.1 to 2.99.2 in /gemfiles/rails_60_turbolinks (activeadmin#5907)

* Bump jasmine-core from 2.9.1 to 2.99.2 in /gemfiles/rails_52 (activeadmin#5909)

* Bump turbolinks from 5.2.0 to 5.2.1 in /gemfiles/rails_60_turbolinks (activeadmin#5905)

* Use $sidebar-with to calculate right-margin of #main_content (activeadmin#5887)

* Use $sidebar-with to calculate right-margin of #main_content

* Add parenthesis to improve readability

Co-Authored-By: Javier Julio <javierjulio@icloud.com>

* Add changelog entry for activeadmin#5887

* Add definition of user and PR to changelog

* Bump sprockets from 3.7.2 to 4.0.0 in /gemfiles/rails_60_turbolinks (activeadmin#5910)

* Replace "Removals" with "Dependency Changes"

Let's save the word "Removals" for removals that we consider to be
"Breaking Changes".

* Remove "Major" and "Minor" subsections

This is quite subjective, and unnecessary complicates the nesting of the
changelog structure.

* Add some missing punctuation

* Quote some codes

* Bump sprockets from 3.7.2 to 4.0.0 (activeadmin#5908)

* Bump github-pages from 192 to 202 in /docs (activeadmin#5897)

* Bump jasmine from 2.9.0 to 2.99.0 in /gemfiles/rails_52 (activeadmin#5915)

* Bump rubocop from 0.75.0 to 0.75.1 (activeadmin#5917)

* Bump jasmine from 2.9.0 to 2.99.0 (activeadmin#5918)

* Bump rubocop-rspec from 1.35.0 to 1.36.0 (activeadmin#5919)

* Bump jasmine from 2.9.0 to 2.99.0 in /gemfiles/rails_60_turbolinks (activeadmin#5916)
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.

2 participants