ManageIQ / manageiq-ui-classic

Classic UI of ManageIQ
Apache License 2.0
50 stars 358 forks source link

Broken travis from other repos #4921

Closed himdel closed 4 years ago

himdel commented 6 years ago

With some frequency, ui-classic travis is red because of changes made in other repos.

This issue exists to track those - please mention this issue in any PR which breaks ui-classic travis (or fixes such a break).

(Please do not use for travis failures not caused by other ManageIQ repos.)

(related to discussion in https://github.com/ManageIQ/manageiq/pull/18173)

Spreadsheet: https://docs.google.com/spreadsheets/d/1V_o2vxaBsHlX70E0SOwoeArmXAZIfQQI8V_JBC8dGvE

Fryguy commented 6 years ago

If, possible, I would love to see the following timings:

Obviously it may not be possible to get all of that, but the more information we have the better we can solve this together.

himdel commented 6 years ago

Red again... https://travis-ci.org/ManageIQ/manageiq-ui-classic/builds/455654431

  1) ComplianceSummaryHelper when @explorer is set #textual_compliance_status
     Failure/Error: {:time => time_ago_in_words(date.in_time_zone(Time.zone)).titleize}
     NoMethodError:
       undefined method `in_time_zone' for nil:NilClass
     # ./app/helpers/compliance_summary_helper.rb:16:in `textual_compliance_status'
     # ./spec/helpers/compliance_summary_helper_spec.rb:22:in `block (3 levels) in <top (required)>'
  2) ComplianceSummaryHelper for classic screens when @explorer is not set #textual_compliance_status
     Failure/Error: {:time => time_ago_in_words(date.in_time_zone(Time.zone)).titleize}
     NoMethodError:
       undefined method `in_time_zone' for nil:NilClass
     # ./app/helpers/compliance_summary_helper.rb:16:in `textual_compliance_status'
     # ./spec/helpers/compliance_summary_helper_spec.rb:50:in `block (3 levels) in <top (required)>'

PR merged: 2018-11-15 16:53 UTC travis went red: 2018-11-15 20:03 UTC discovered: 2018-11-16 13:30 UTC (me, presumably sooner by others?; 12:23 UTC on Brno gitter) core caused: about 30 minutes, https://github.com/ManageIQ/manageiq/pull/17475 (Thanks @Hyperkid123 for finding the problem (can you add how much time you spent on it too please?)) EDIT: so, an hour

fix created: 2018-11-16 14:10 UTC (assuming https://github.com/ManageIQ/manageiq-ui-classic/pull/4933) PR merged: 2018-11-16 19:06 UTC travis went green: 2018-11-16 19:42 UTC (https://travis-ci.org/ManageIQ/manageiq-ui-classic/builds/456115082)

Hyperkid123 commented 6 years ago

Thanks @Hyperkid123 for finding the problem (can you add how much time you spent on it too please?

It took me about 15-20 minutes. And about 30 minutes until i realized that the tests are busted (don't know if that counts). Thanks for the fix @himdel !

skateman commented 6 years ago

This could be nicely automated by just storing some information about PR merges and travis build results from all the related repos. If the PR's build was green and the master branch afterwards went red, there's probably an issue in the other repositories. This is way too interesting to solve during working hours, if no one wants it, I might play with this during the holidays in December.

bdunne commented 6 years ago

@skateman That does sound like a fun project :smile:

martinpovolny commented 5 years ago

@bdunne, @Fryguy:

on the argument of long running UI tests: We fixed that.

test-run-fast

himdel commented 5 years ago

Another red...

https://travis-ci.org/ManageIQ/manageiq-ui-classic/jobs/474623058

  1) PhysicalInfraTopologyService#build_topology topology contains the expected structure and content
     Failure/Error:
       expect(subject[:items]).to eq(
         "PhysicalInfraManager" + ems.id.to_s  => {
           :name         => ems.name,
           :kind         => "PhysicalInfraManager",
           :miq_id       => ems.id,
           :status       => nil,
           :display_kind => "Lenovo",
           :model        => ems.class.name,
           :key          => "PhysicalInfraManager" + ems.id.to_s
         },
       expected: {"PhysicalInfraManager96000000000077"=>{:name=>"LXCA", :kind=>"PhysicalInfraManager", :miq_id=>960000...PhysicalServer", :provider=>"LXCA", :model=>"PhysicalServer", :key=>"PhysicalServer96000000000004"}}
            got: {"PhysicalInfraManager96000000000077"=>{:name=>"LXCA", :kind=>"PhysicalInfraManager", :model=>"Manage..."PhysicalRack96000000000001", :status=>"Unknown", :display_kind=>"PhysicalRack", :provider=>"LXCA"}}

(hidden in the diff, but expected has :status => nil, while actual has :status => 'Valid').

Comes from https://github.com/ManageIQ/manageiq/pull/18266. Fixed by https://github.com/ManageIQ/manageiq-ui-classic/pull/5120

Merged: 2019-01-02 15:17Z Went red: 2019-01-02 16:51Z Discovered: 2019-01-03 10:53Z Fix created: 2019-01-03 12:27Z PR merged: 2019-01-03 12:49Z Went green: 2019-01-03 13:03Z

cben commented 5 years ago

Thanks, now I know what broke providers-kubernetes tests, being subscribed to this issue saves a lot of time debugging :wink:

Fryguy commented 5 years ago

I made a spreadsheet... link in the OP.

himdel commented 5 years ago

Another..

https://travis-ci.org/ManageIQ/manageiq-ui-classic/jobs/485263617

  1) TopologyService#entity_name entity is a tag returns with the parent and the child classification description
     Failure/Error: cond.exists?
     SystemStackError:
       stack level too deep
     # ./spec/manageiq/app/models/classification.rb:16:in `block in <class:Classification>'
     # ./spec/manageiq/app/models/classification.rb:533:in `save_tag'
...
     # ./spec/manageiq/app/models/classification.rb:533:in `save_tag'
     # ./spec/manageiq/spec/support/missing_factory_helper.rb:10:in `create'

Caused by https://github.com/ManageIQ/manageiq/pull/18378

Fixed... ?

(added dates to spreadsheet)

Fryguy commented 5 years ago

We just reverted ManageIQ/manageiq#18378 with https://github.com/ManageIQ/manageiq/pull/18408, so I'm considering that "fixed"

himdel commented 5 years ago

https://travis-ci.org/ManageIQ/manageiq-ui-classic/jobs/498918915 caused by https://github.com/ManageIQ/manageiq/pull/18486 fixed by https://github.com/ManageIQ/manageiq-ui-classic/pull/5285

Failures:
  1) ChargebackController chargeback rate form actions with default chargebacks rates adds new chargeback rate using default values
     Failure/Error: expect(rate_detail.detail_currency.name).to eq(rate_detail_hash[:type_currency])
       expected: "USD"
            got: "United States Dollar"
       (compared using ==)
     # ./spec/controllers/chargeback_controller_spec.rb:385:in `block in expect_chargeback_rate_to_eq_hash'
     # ./spec/controllers/chargeback_controller_spec.rb:381:in `each_with_index'
     # ./spec/controllers/chargeback_controller_spec.rb:381:in `expect_chargeback_rate_to_eq_hash'
     # ./spec/controllers/chargeback_controller_spec.rb:421:in `block (4 levels) in <top (required)>'
  2) ChargebackController chargeback rate form actions with default chargebacks rates adds new chargeback rate and user adds and removes some tiers
     Failure/Error: expect(rate_detail.detail_currency.name).to eq(rate_detail_hash[:type_currency])
       expected: "USD"
            got: "United States Dollar"
       (compared using ==)
     # ./spec/controllers/chargeback_controller_spec.rb:385:in `block in expect_chargeback_rate_to_eq_hash'
     # ./spec/controllers/chargeback_controller_spec.rb:381:in `each_with_index'
     # ./spec/controllers/chargeback_controller_spec.rb:381:in `expect_chargeback_rate_to_eq_hash'
     # ./spec/controllers/chargeback_controller_spec.rb:451:in `block (4 levels) in <top (required)>'

Thanks @lpichler :)

agrare commented 5 years ago

https://travis-ci.org/ManageIQ/manageiq/jobs/500425821#L2517-L2524 caused by https://github.com/ManageIQ/manageiq-ui-classic/pull/5271 fixed by https://github.com/ManageIQ/manageiq/pull/18478

Failures:
  1) MiqWidget.import_from_hash rss feed internal with new rss feed import
     Failure/Error: WidgetImportService.new.import_widget_from_hash(widget)

     NoMethodError:
       undefined method `new_record?' for nil:NilClass
     # ./app/models/miq_widget/import_export.rb:8:in `import_from_hash'
     # ./spec/models/miq_widget/import_from_hash_spec.rb:27:in `block (3 levels) in <top (required)>'
     # ./spec/models/miq_widget/import_from_hash_spec.rb:135:in `block (6 levels) in <top (required)>'

This issue tracks failures in the other direction too right? :smile:

Fryguy commented 5 years ago

This issue tracks failures in the other direction too right? 😄

You mean because a UI PR was merged which broke core?

agrare commented 5 years ago

You mean because a UI PR was merged which broke core

Yes

himdel commented 5 years ago

https://travis-ci.org/ManageIQ/manageiq-ui-classic/jobs/519439431

Failures:
  1) AuthKeyPairCloudController#report_data when tile mode is selected returns key pairs with quadicons
     Failure/Error: expect(results['data']['rows'][0]['quad']).to have_key('fonticon')
       expected  to respond to `has_key?`
     # ./spec/controllers/auth_key_pair_cloud_controller_spec.rb:98:in `block (4 levels) in <top (required)>'

cause https://github.com/ManageIQ/manageiq/pull/18633 fix https://github.com/ManageIQ/manageiq-decorators/pull/4

himdel commented 5 years ago

https://travis-ci.org/ManageIQ/manageiq-ui-classic/builds/526150604

  1) CatalogController tests that needs all rbac features access #st_upload_image uploads a selected png file 
     Failure/Error: expect(assigns(:flash_array).first[:message]).to include('Custom Image file "upload_image.png" successfully uploaded')

     NoMethodError:
       undefined method `first' for nil:NilClass
     # ./spec/controllers/catalog_controller_spec.rb:287:in `block (4 levels) in <top (required)>'

rspec ./spec/controllers/catalog_controller_spec.rb:284

cause https://github.com/ManageIQ/manageiq/pull/18689 fix https://github.com/ManageIQ/manageiq/pull/18705

himdel commented 5 years ago

https://travis-ci.org/ManageIQ/manageiq-ui-classic/jobs/533531882#L2666

 1) MiqAeClassController save class/method update a method with inputs
     Failure/Error: render :json => ex.for_render

     Module::DelegationError:
       ActionController::Metal#content_type delegated to @_response.content_type, but @_response is nil.......
     # ./app/helpers/application_helper/flash.rb:27:in `javascript_flash'
     # ./app/controllers/miq_ae_class_controller.rb:1235:in `rescue in update_method'
     # ./app/controllers/miq_ae_class_controller.rb:1223:in `update_method'
     # ./spec/controllers/miq_ae_class_controller_spec.rb:575:in `block (3 levels) in <top (required)>'
     # ------------------
     # --- Caused by: ---
     # ActiveRecord::RecordInvalid:
     #   Validation failed: MiqAeMethod: Embedded methods is reserved
     #   ./app/controllers/miq_ae_class_controller.rb:1229:in `block in update_method'

  2) MiqAeClassController#ae_class_validation Should not allow to add two parameters with identical name to a newly created method
     Failure/Error: expect(assigns(:flash_array).first[:message]).to include("Name has already been taken")
       expected "Error during 'add': Validation failed: MiqAeMethod: Embedded methods is reserved" to include "Name has already been taken"
     # ./spec/controllers/miq_ae_class_controller_spec.rb:534:in `block (3 levels) in <top (required)>'

cause https://github.com/ManageIQ/manageiq/pull/18778 fix https://github.com/ManageIQ/manageiq-ui-classic/pull/5601

mzazrivec commented 5 years ago
1) CloudTenantController#create queues the create action
     Failure/Error: MiqTask.generic_action_with_callback(task_opts, queue_opts)
       #<MiqTask(id: integer, name: string, state: string, status: string, message: text, userid: string, created_on: datetime, updated_on: datetime, pct_complete: integer, context_data: text, results: text, miq_server_id: integer, identifier: string, started_on: datetime, zone: string, href_slug: string, region_number: integer, region_description: string, state_or_status: string) (class)> received :generic_action_with_callback with unexpected arguments
         expected: ({:action=>"creating Cloud Tenant for user user2897", :userid=>"user2897"}, {:args=>[11000000002025, {:name=>"foo"}], :class_name=>ManageIQ::Providers::Openstack::CloudManager::... ), :method_name=>"create_cloud_tenant", :priority=>20, :role=>"ems_operations", :zone=>"Zone 3000"})
              got: ({:action=>"creating Cloud Tenant for user user2897", :userid=>"user2897"}, {:args=>[11000000002025, {:name=>"foo"}], :class_name=>"ManageIQ::Providers::Openstack::CloudManager:...t", :method_name=>"create_cloud_tenant", :priority=>20, :role=>"ems_operations", :zone=>"Zone 3000"})
       Diff:
       @@ -1,7 +1,6 @@
        [{:action=>"creating Cloud Tenant for user user2897", :userid=>"user2897"},
         {:args=>[11000000002025, {:name=>"foo"}],
       -  :class_name=>
       -   ManageIQ::Providers::Openstack::CloudManager::CloudTenant(id: integer, name: string, description: text, enabled: boolean, ems_ref: string, ems_id: integer, created_at: datetime, updated_at: datetime, type: string, parent_id: integer, href_slug: string, region_number: integer, region_description: string, total_vms: integer, cloud_volume_types: ),
       +  :class_name=>"ManageIQ::Providers::Openstack::CloudManager::CloudTenant",
          :method_name=>"create_cloud_tenant",
          :priority=>20,
          :role=>"ems_operations",
     # ./spec/manageiq/app/models/cloud_tenant.rb:67:in `create_cloud_tenant_queue'
     # ./app/controllers/cloud_tenant_controller.rb:87:in `create'
     # ./spec/controllers/cloud_tenant_controller_spec.rb:120:in `block (3 levels) in <top (required)>'

Cause: https://github.com/ManageIQ/manageiq/pull/18827 Fix: https://github.com/ManageIQ/manageiq-ui-classic/pull/5660

himdel commented 5 years ago

https://travis-ci.org/ManageIQ/manageiq-ui-classic/jobs/544108049

  1) ReportHelper#chart_fields_options should return numeric fields from report with models when "Show Sort Breaks" is "No"
     Failure/Error: data_type = parse_field_or_tag(path).try(:column_type)

     NoMethodError:
       undefined method `parse_field_or_tag' for #<Class:0x000000000a783ea8>
     # ./spec/manageiq/app/models/miq_report.rb:127:in `get_col_info'
     # ./app/helpers/report_helper.rb:53:in `block in chart_fields_options'
     # ./app/helpers/report_helper.rb:52:in `each'
     # ./app/helpers/report_helper.rb:52:in `find_all'
     # ./app/helpers/report_helper.rb:52:in `chart_fields_options'
     # ./spec/helpers/report_helper_spec.rb:57:in `block (3 levels) in <top (required)>'

Cause: https://github.com/ManageIQ/manageiq/pull/18784 Fix: https://github.com/ManageIQ/manageiq/pull/18850

himdel commented 5 years ago

https://travis-ci.org/ManageIQ/manageiq-ui-classic/jobs/544619532

Failures:
  1) AnsibleCredentialController#show renders correct template and listnav
     Failure/Error: is_expected.to render_template(:partial => "layouts/listnav/_ansible_credential")
       expecting partial <layouts/listnav/_ansible_credential> but action rendered <["layouts/_flash_msg", "_flash_msg", "layouts/_textual_groups_generic", "_textual_groups_generic", "layouts/_exception_contents", "_exception_contents", "layouts/_doctype", "_doctype", "stylesheets/_template50", "_template50", "layouts/_i18n_js", "_i18n_js", "layouts/_user_options", "_user_options", "layouts/_notifications_drawer", "_notifications_drawer", "layouts/_toast_list", "_toast_list", "layouts/_spinner", "_spinner", "layouts/_lightbox_panel", "_lightbox_panel", "layouts/_header", "_header", "layouts/_vertical_navbar", "_vertical_navbar", "layouts/_breadcrumbs", "_breadcrumbs", "layouts/_center_div_no_listnav", "_center_div_no_listnav", "layouts/_content", "_content", "layouts/_adv_search_body", "_adv_search_body", "layouts/_adv_search_footer", "_adv_search_footer", "layouts/_adv_search", "_adv_search", "layouts/_footer", "_footer"]>.
       Expected {"layouts/_flash_msg"=>1, "_flash_msg"=>1, "layouts/_textual_groups_generic"=>1, "_textual_groups_generic"=>1, "layouts/_exception_contents"=>1, "_exception_contents"=>1, "layouts/_doctype"=>1, "_doctype"=>1, "stylesheets/_template50"=>1, "_template50"=>1, "layouts/_i18n_js"=>1, "_i18n_js"=>1, "layouts/_user_options"=>1, "_user_options"=>1, "layouts/_notifications_drawer"=>1, "_notifications_drawer"=>1, "layouts/_toast_list"=>1, "_toast_list"=>1, "layouts/_spinner"=>1, "_spinner"=>1, "layouts/_lightbox_panel"=>1, "_lightbox_panel"=>1, "layouts/_header"=>1, "_header"=>1, "layouts/_vertical_navbar"=>1, "_vertical_navbar"=>1, "layouts/_breadcrumbs"=>1, "_breadcrumbs"=>1, "layouts/_center_div_no_listnav"=>1, "_center_div_no_listnav"=>1, "layouts/_content"=>1, "_content"=>1, "layouts/_adv_search_body"=>1, "_adv_search_body"=>1, "layouts/_adv_search_footer"=>1, "_adv_search_footer"=>1, "layouts/_adv_search"=>1, "_adv_search"=>1, "layouts/_footer"=>1, "_footer"=>1} to include "layouts/listnav/_ansible_credential".
     # ./spec/controllers/ansible_credential_controller_spec.rb:14:in `block (3 levels) in <top (required)>'

(uninitialized constant ManageIQ::Providers::EmbeddedAnsible::AutomationManager::MachineCredential::API_ATTRIBUTES [ansible_credential/show])

Cause: https://github.com/ManageIQ/manageiq/pull/18687 Fix: https://github.com/ManageIQ/manageiq/pull/18854

lpichler commented 5 years ago

(NOT UI)Repo: https://github.com/ManageIQ/manageiq-api

https://travis-ci.org/ManageIQ/manageiq-api/jobs/545719188

Failures:
  1) Requests API request creation can access attributes of its workflow
     Failure/Error: request.add_tag(t.name, t.children.first.name)

     NoMethodError:
       undefined method `name' for nil:NilClass
     # ./spec/requests/requests_spec.rb:293:in `block (3 levels) in <top (required)>'
  2) Requests API request creation exposes various attributes in the request resources
     Failure/Error: request.add_tag(t.name, t.children.first.name)

     NoMethodError:
       undefined method `name' for nil:NilClass
     # ./spec/requests/requests_spec.rb:262:in `block (3 levels) in <top (required)>'

Cause: https://github.com/ManageIQ/manageiq/pull/18301 Fix: https://github.com/ManageIQ/manageiq-api/pull/610

himdel commented 5 years ago

https://travis-ci.org/ManageIQ/manageiq-ui-classic/jobs/547264151

DevTools listening on ws://127.0.0.1:9222/devtools/browser/ab493a73-18c8-43db-9b46-b0595e5fb579
.................................................................................................2019-06-18 14:53:59 +0000: Rack app error handling request { GET /static/verify-button.html.haml }
#<NameError: uninitialized constant StaticOrHaml::Haml>
/home/travis/build/ManageIQ/manageiq-ui-classic/spec/javascripts/support/jasmine_helper.rb:24:in `call'
/home/travis/build/ManageIQ/manageiq-ui-classic/vendor/bundle/gems/rack-2.0.7/lib/rack/urlmap.rb:68:in `block in call'
/home/travis/build/ManageIQ/manageiq-ui-classic/vendor/bundle/gems/rack-2.0.7/lib/rack/urlmap.rb:53:in `each'
/home/travis/build/ManageIQ/manageiq-ui-classic/vendor/bundle/gems/rack-2.0.7/lib/rack/urlmap.rb:53:in `call'

Cause: https://github.com/ManageIQ/manageiq/pull/18886 Fix: https://github.com/ManageIQ/manageiq-ui-classic/pull/5721

himdel commented 5 years ago

https://travis-ci.org/ManageIQ/manageiq-ui-classic/jobs/547264149

  17) ContainerImageRegistryController download pdf file request returns 200
      Failure/Error: MiqServer.my_server.name
      ActionView::Template::Error:
        undefined method `name' for nil:NilClass
      Shared Example Group: "#download_summary_pdf" called from ./spec/controllers/container_image_registry_controller_spec.rb:58
      # ./app/helpers/application_helper.rb:1238:in `appliance_name'
      # ./app/views/layouts/_user_options.html.haml:10:in `__home_travis_build__anage___manageiq_ui_classic_app_views_layouts__user_options_html_haml__896313301279762601_356502600'
      # ./app/views/layouts/_header.html.haml:32:in `__home_travis_build__anage___manageiq_ui_classic_app_views_layouts__header_html_haml__493180963747152278_358287940'
      # ./app/views/layouts/application.html.haml:32:in `__home_travis_build__anage___manageiq_ui_classic_app_views_layouts_application_html_haml___735393451250615652_289871660'
      # ./app/controllers/application_controller.rb:280:in `block (2 levels) in render_exception'
      # ./app/controllers/application_controller.rb:268:in `render_exception'
      # ./app/controllers/application_controller.rb:263:in `error_handler'
      # ./spec/shared/controllers/shared_example_for_download_summary_pdf.rb:12:in `block (3 levels) in <top (required)>'
      # ------------------
      # --- Caused by: ---
      # Sprockets::FileNotFound:
      #   couldn't find file '@manageiq/ui-components/dist/css/ui-components' with type 'text/css'

Cause: https://github.com/ManageIQ/manageiq/pull/18869 Fix: https://github.com/ManageIQ/manageiq-ui-classic/pull/5722

himdel commented 5 years ago

https://travis-ci.org/ManageIQ/manageiq-ui-classic/jobs/547432152

8) YAML reports regular reports 900_Provisioning - Activity Reports/140_Provisioning Activity - by VM can be built even though without data
     Failure/Error:
       scope = scope.from(Arel.sql("(#{inner_scope.to_sql})").as(scope.table_name))
                    .except(:offset, :limit, :order, :where)
     ActiveRecord::ConfigurationError:
       Can't join 'ExtManagementSystem' to association named 'hostname'; perhaps you misspelled it?
     Shared Example Group: "regular report examples" called from ./spec/product/reports_spec.rb:64
     # ./spec/manageiq/lib/rbac/filterer.rb:276:in `search'
     # ./spec/manageiq/lib/rbac/filterer.rb:147:in `search'
     # ./spec/manageiq/lib/rbac.rb:3:in `search'
     # ./spec/manageiq/app/models/miq_report/generator.rb:323:in `generate_basic_results'
     # ./spec/manageiq/app/models/miq_report/generator.rb:200:in `_generate_table'
     # ./spec/manageiq/app/models/miq_report/generator.rb:181:in `block in generate_table'
     # ./spec/manageiq/app/models/user.rb:268:in `with_user'
     # ./spec/manageiq/app/models/miq_report/generator.rb:181:in `generate_table'
     # ./spec/product/reports_spec.rb:40:in `block (4 levels) in <top (required)>'

Cause: https://github.com/ManageIQ/manageiq/pull/18822 Fix: https://github.com/ManageIQ/manageiq/pull/18892

himdel commented 5 years ago

https://travis-ci.org/ManageIQ/manageiq-ui-classic/builds/551515228

  1) shared/views/ems_common/show when display is 'cloud_volumes' should show render gtl for list of cloud_volumes
     Failure/Error: authentications.select { |a| a.kind_of?(AuthUseridPassword) }
     NoMethodError:
       private method `select' called for nil:NilClass
     # ./spec/manageiq/app/models/mixins/authentication_mixin.rb:63:in `authentication_userid_passwords'
     # ./spec/manageiq/app/models/mixins/authentication_mixin.rb:450:in `available_authentications'
     # ./spec/manageiq/app/models/mixins/authentication_mixin.rb:234:in `authentication_type'
     # ./spec/manageiq/app/models/mixins/authentication_mixin.rb:438:in `authentication_best_fit'
     # ./spec/manageiq/app/models/mixins/authentication_mixin.rb:135:in `auth_user_pwd'
     # ./spec/manageiq/app/models/mixins/authentication_mixin.rb:157:in `update_authentication'
     # ./spec/views/shared/views/ems_common/_show.html.haml_spec.rb:16:in `block (3 levels) in <top (required)>'
  2) EmsNetworkController for azure #show renders show screen
     Failure/Error: expect(response.status).to eq(200)
       expected: 200
            got: 500
       (compared using ==)
     Shared Example Group: :shared_examples_for_ems_network_controller called from ./spec/controllers/ems_network_controller_spec.rb:2
     # ./spec/shared/controllers/shared_examples_for_ems_network_controller.rb:36:in `block (5 levels) in <top (required)>'
  3) EmsCloudHelper::TextualSummary#textual_description EMS instance doesn't support :description #textual_description should eq nil
     Failure/Error: it { is_expected.to eq(value) }
       expected: nil
            got: {:label=>"Description", :value=>"US East (N. Virginia)"}
       (compared using ==)
     Shared Example Group: "textual_description" called from ./spec/helpers/ems_cloud_helper/textual_summary_spec.rb:65
     # ./spec/shared/helpers/textual_summary_helper_methods.rb:38:in `block (3 levels) in <top (required)>'

Cause: https://github.com/ManageIQ/manageiq/pull/18842 Fix: ???

bdunne commented 5 years ago

cc @d-m-u for https://github.com/ManageIQ/manageiq-ui-classic/issues/4921#issuecomment-506736795

himdel commented 5 years ago

:+1: comes from https://github.com/ManageIQ/manageiq/pull/18842

d-m-u commented 5 years ago

Yeah, yeah, sorry @himdel, working on it

martinpovolny commented 5 years ago

Do we have enough data to argue for or agains running the UI tests in core PRs? Do we need to carry on with this exercise?

@himdel ? @Fryguy ? @jrafanie ?

himdel commented 5 years ago

IMO we have enough data to say that we absolutely need something. In fact, I now think the next release is in danger because of all these failures and the time & energy this saps away from me every single week (it's not, provided nobody does it again, but ..what are the chances :( ).

(Not sure if just running UI tests in core is the solution though, mostly because there's also all the provider repos, manageiq-content, manageiq-schema, etc.)

himdel commented 5 years ago

https://travis-ci.org/ManageIQ/manageiq-ui-classic/jobs/552512905


  1) MiqAeCustomizationController#upload_import_file when an upload file is given when the dialog importer raises a circular reference error redirects with an error message
     Failure/Error:
       allow(dialog_import_service).to receive(:store_for_import)
         .and_raise(DialogImportValidator::DialogFieldAssociationCircularReferenceError)
     NameError:
       uninitialized constant DialogImportValidator::DialogFieldAssociationCircularReferenceError
     # ./spec/controllers/miq_ae_customization_controller_spec.rb:292:in `block (5 levels) in <top (required)>'
  2) MiqAeCustomizationController#upload_import_file when an upload file is given when the dialog importer raises an invalid dialog field type error redirects with an error message
     Failure/Error: rescue DialogImportValidator::DialogFieldAssociationCircularReferenceError
     NameError:
       uninitialized constant DialogImportValidator::DialogFieldAssociationCircularReferenceError
     # ./app/controllers/miq_ae_customization_controller.rb:52:in `rescue in upload_import_file'
     # ./app/controllers/miq_ae_customization_controller.rb:43:in `upload_import_file'
     # ./spec/controllers/miq_ae_customization_controller_spec.rb:324:in `block (5 levels) in <top (required)>'
     # ------------------
     # --- Caused by: ---
     # DialogImportValidator::InvalidDialogFieldTypeError:
     #   DialogImportValidator::InvalidDialogFieldTypeError
     #   ./app/controllers/miq_ae_customization_controller.rb:44:in `upload_import_file'
  3) EmsCloudHelper::TextualSummary#textual_description EMS instance doesn't support :description #textual_description should eq nil
     Failure/Error: it { is_expected.to eq(value) }
       expected: nil
            got: {:label=>"Description", :value=>"US East (N. Virginia)"}
       (compared using ==)
     Shared Example Group: "textual_description" called from ./spec/helpers/ems_cloud_helper/textual_summary_spec.rb:65
     # ./spec/shared/helpers/textual_summary_helper_methods.rb:38:in `block (3 levels) in <top (required)>'
  4) shared/views/ems_common/show when display is 'cloud_volumes' should show render gtl for list of cloud_volumes
     Failure/Error: authentications.select { |a| a.kind_of?(AuthUseridPassword) }
     NoMethodError:
       private method `select' called for nil:NilClass
     # ./spec/manageiq/app/models/mixins/authentication_mixin.rb:63:in `authentication_userid_passwords'
     # ./spec/manageiq/app/models/mixins/authentication_mixin.rb:450:in `available_authentications'
     # ./spec/manageiq/app/models/mixins/authentication_mixin.rb:234:in `authentication_type'
     # ./spec/manageiq/app/models/mixins/authentication_mixin.rb:438:in `authentication_best_fit'
     # ./spec/manageiq/app/models/mixins/authentication_mixin.rb:135:in `auth_user_pwd'
     # ./spec/manageiq/app/models/mixins/authentication_mixin.rb:157:in `update_authentication'
     # ./spec/views/shared/views/ems_common/_show.html.haml_spec.rb:16:in `block (3 levels) in <top (required)>'

Cause: https://github.com/ManageIQ/manageiq/pull/18922 + https://github.com/ManageIQ/manageiq/pull/18890 Fix: https://github.com/ManageIQ/manageiq-ui-classic/pull/5755

2 of those failures are already present in https://travis-ci.org/ManageIQ/manageiq-ui-classic/jobs/551693217, so this is likely caused by at least 2 PRs.

d-m-u commented 5 years ago

@himdel both fixed here: https://github.com/ManageIQ/manageiq-ui-classic/pull/5755

himdel commented 5 years ago

Thanks! :) Next time, ping me in the PR please? People had their PRs broken for 3 days because nobody knew about your fix :)

bdunne commented 5 years ago

IMO we have enough data to say that we absolutely need something.

I'm still not convinced, in general these are pretty rare (~1/month), but in the past month it has happened more frequently due to some changes going on in core. If you want to more carefully introduce core changes to the UI repo, maybe change https://github.com/ManageIQ/manageiq-ui-classic/blob/master/bin/setup#L8 from branch to a specific sha and update that daily?

himdel commented 5 years ago

@bdunne Consider that ~20 people have their PRs red for half a day every time (well, maybe not 20 these days, with the effort split between the repos, but still, everybody in the UI team.) I would argue that each such failure wastes about 1-2 man-days of time. Also, all merging stops.

Not to mention, nobody else is dealing with these and I really want to be able to do something else. I'm this close to just letting travis be red.

So ... no, not doing anything is not feasible or acceptable anymore, sorry.

EDIT: the idea of the UI not using manageiq master branch will just lead to the UI depending on a random old version of manageiq with no easy way of ever getting it back up to date, so.. no.

himdel commented 5 years ago

And just to be clear, I don't think the problem is really that we don't catch the failures before they get merged. We're a big project and we can probably live with that.

The problem is that when that happens, there's no automation, so this is always:

So... I don't think we need to be running ui-classic travis on every single core&provider PR.

IMO we just need something that runs all the repos' tests regularly, detects failures, and narrows those down to a specific PR, and pings the author of that PR, preferably within an hour of their PR getting merged.

martinpovolny commented 5 years ago

@bdunne : if you are not convinced than we are probably not doing a good job explaining how bad the situation looks from our side.

Let me assign some emotions to that: tired, helpless, tired again, some more tired, distracted, hopeless.

@himdel is spending by far too much time on it. Maybe the cure is for him to stop doing it completely.

himdel commented 5 years ago

BTW, looking at the times now..

manageiq-ui-classic spec - 18 minutes manageiq-api - 14 minutes manageiq - 20 minutes

that's less than an hour, so, it would actually be possible to run ui & api specs in core.

(I mean, that's not my top solution, but simplicity is definitely an argument for it.)

Oh, and the API has been red for 5 days now :(.

Fryguy commented 5 years ago

Oh, and the API has been red for 5 days now :(.

Assuming you're not counting weekend, that's unacceptable, IMO. We should have reverted whatever caused that long ago.

agrare commented 5 years ago

This looks like the first failed run (Thursday) https://travis-ci.org/ManageIQ/manageiq-api/builds/551428230

himdel commented 5 years ago

Sorry, that's including the weekend, not bussiness days. (Really, it's the human date travis said for the last run that was greeen.)

Still, agreed there :)

martinpovolny commented 5 years ago

Guys, pls, do not hijack the topic.

Fryguy commented 5 years ago

So, we've had this repo for about about 8 months now, and I think the recording of events has helped a lot. Except for these past ~2-3 weeks, the incidents occur roughly once every 2-3 weeks, with 0 events occurring in March. These particular last 2 weeks have been especially bad, and I'd like to understand specifically why before taking action, but according to the data it seems more of an anomaly.

On the merging side of core, these past couple weeks things have been merged even after we stated in the PR, "we should manually run the UI and API tests here just to be sure". That on us to do better, for sure. No excuses there. Additionally, we should have triggered a double check on any of the ones where factories were modified. Those are almost guaranteed to cause extra-repo failures.

A while ago I proposed a compromise solution. Based on the results of the spreadsheet in this PR, I feel like my initial assessment is the same. That is, I agree with @himdel, that we need to do something, but I still don't think this warrants running all of the test suites on every core PR. My proposal (can't find a link now), is that we should have a bot command that runs whatever tests we ask it too. IMO, this would have caught all of those occasions listed above where we said ""we should manually run the UI and API tests".

So, roughly, I envision in a PR one could do @miq-bot run_tests manageiq-ui-classic. We would have a dedicated travis repo where these tests run, and essentially what it would do is clone manageiq-ui-classic master, set up the spec/manageiq directory to be the branch of the core PR, and then run the tests. The bot would monitor this and post the results back to the PR when done.

This would be the most basic setup. I have a full design somewhere (need to find it), where I had commands for running other repos, as well as running the PR in the context of other changes (for example, if you change a gem, and want to run your PR in the context of the gem change). Obviously all future stuff.

I could also see the bot set up to run something on an interval (unfortunately, the cron command on travis only goes as granular as daily). Additionally, I could see the bot marking a PR as "rejected" if particular files are touched (such as factories), and only approves if someone runs the necessary tests.

IMO we just need something that runs all the repos' tests regularly

Travis already has this, and it's currently set up to run daily...unfortunately that's the most granular. Otherwise we need the bot to do it more frequently via API or something.

martinpovolny commented 5 years ago

but I still don't think this warrants running all of the test suites on every core PR.

I disagree. I think that we really should run the Ruby UI tests suite with a single Ruby version on each PR on a number of repositories including core and the main providers at least. That is 15 minutes.

The UI is the integration piece. It is the closest we have to the customer experience. And it's what eventually matters.

I see the value of human time and human nerves (@himdel 's in this case) very high and I think that the current situation is not acceptable.

However I respect that you do not want to go that way and even @himdel does not want to go that way. I am in a minority.

On the solution to the problem:

So, roughly, I envision in a PR one could do @miq-bot run_tests manageiq-ui-classic. We would have a dedicated travis repo where these tests run, and essentially what it would do is clone manageiq-ui- classic master, set up the spec/manageiq directory to be the branch of the core PR, and then run the tests. The bot would monitor this and post the results back to the PR when done.

Yes a dedicated repo in travis where we run the tests. That is good.

I could also see the bot set up to run something on an interval

A cron script that commits to the repo every hour to trigger a build.

After the build another commit would be made with the status of the build and a list of PRs that where merged in the depencency repos and the status of the build. It would be done in such a way that there would exist a clickable page (in the repo) where one could easily start the search project to find the cause of the failure.

Doing that every hour we will have a decent granularity (that can be tweeked), a place to start the investigation and possibly a nice chart showing the status over time.

After the feature freeze I'll just do that and end this discussion.

skateman commented 5 years ago

I'm a little worried that a dedicated repo would just go red because we did some change in the way how we run our tests and start producing false-negative results. Also it's a little problemmatic to set it up.

@martinpovolny what about having just a branch in the ui repo that gets updated (and rebased against master) upon any merged PR in any other repo? I am pretty sure this is possible via webhooks and it only requires to set these webhooks in all the other repos we're depending on. The webhooks would update a list of commits in separate files representing the dependent repos.

This way we could narrow down the list of commits that are breaking our travis by just looking into these files and into the build history of the branch. However, I'm not familiar with the quotas we have on travis, if it is on per branch or per organization. So we might end up with slower tests compared to a separate repo. cc @Fryguy

Fryguy commented 5 years ago

So last night I put something together, and the basics work. I'll try and get that published today, and hopefully get the bot integration done as well.

Fryguy commented 5 years ago

I'm not familiar with the quotas we have on travis, if it is on per branch or per organization.

15 simultaneous jobs for the entire org (we paid up...normally it's 5), where a job is 1 line-item in a build (e.g. This has 4 jobs)

skateman commented 5 years ago

This means that it doesn't matter what we use: a repo or a branch.

Fryguy commented 5 years ago

@skateman I am curious about the narrowing down commits bit, so I'd like to understand that more. Considering that the ui-classic repo already has a cron set up on a daily basis, at most you have a days worth of commits. Actual time of execution is about 5:45PM Eastern. I would have thought you'd only need to look at failures with that time point as a boundary.