Manageiq-ui-classic: Broken travis from other repos

[himdel]

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]

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

• Time when ui-classic went red
• Time when it was discovered that ui-classic went red (helps us understand TZ complications where it goes red in the middle of your night, and then not discovering that until the morning)
• Time when it was determined that core caused the red
• Time when PRs are created to fix it
• Time when PRs are merged
• Time when ui-classic goes green

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]

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]

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]

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]

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

[martinpovolny]

@bdunne, @Fryguy:

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

[himdel]

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]

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

[Fryguy]

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

[himdel]

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]

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

[himdel]

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]

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]

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

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

[agrare]

You mean because a UI PR was merged which broke core

Yes

[himdel]

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]

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]

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]

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]

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]

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]

(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]

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]

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]

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]

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]

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

[himdel]

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

[d-m-u]

Yeah, yeah, sorry @himdel, working on it

[martinpovolny]

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]

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]

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]

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

[himdel]

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

[bdunne]

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]

@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]

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:

• somebody tells me travis is red
• I go looking for since when,
• I go through likely-sounding merges in all the 20 manageiq repos for that time
• I try to find the PR that broke it
• after that, the author usually fixes that, or sometimes one of us here does

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]

@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]

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]

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]

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

[himdel]

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]

Guys, pls, do not hijack the topic.

[Fryguy]

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]

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]

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]

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]

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]

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

[Fryguy]

@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.

[Fryguy]

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?

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

Right...because of the shared jobs, any way you slice it, it's still 15 minutes extra per PR. which is 15 minute another job can't run. This way doesn't hold up the author of the PR, but we still pay the time penalty as an org.

[skateman]

@Fryguy you create a folder that will contain files representing repo names. Each repo's webhook will modify the file related to itself, upon each merge to its master it would add a new line with the merged PR number and push it. Maybe you need to add a few newlines to avoid merge conflicts.

If a build fails, you'd clearly see a failure in the branch history and if you look at the commit, you'll see what merged PR in what repo caused the failure.

[cben]

Half baked thoughts: maybe a branch with Gemfile.lock included, which is easies to consume by both Travis and devs?

May be harder to implement though? The robust way is generate Gemfile.lock by actually running bundle install, but that's wasteful if done outside only to generate a commit that'll run bundle install again inside Travis...

But I don't really care how the input triggering tests looks like. The payoff would be a 2nd branch/fork with Gemfile.lock pushed only when tests pass, which humans can stay on while master is risky.
Though one doesn't want to make PRs based on that, or we risk committing Gemfile.lock to main repo 🤔

[cben]

FWIW, long ago I started work on a tool https://github.com/patch-graph/pass-the-fork for testing dependent repos on Travis with gems replaced by PR branches.
It uses Travis API to run a modified test without a commit!

It just recently reached a point where it's also capble of injecting manageiq core into UI/providers (unlike other repos, core is not consumed as a gem but git clone).
It's still not user-friendly and _completely_ undocumented 😳

I'm not claiming it's the right tool, but the techniques might help...

[himdel]

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.

That's still 10 PRs on average, across the 60 repos or so (less, I'm counting all manageiq* repos).

[Fryguy]

across the 60 repos or so (less, I'm counting all manageiq* repos).

Is it that common that a non-core repo also breaks the UI? I thought the main issue was that core itself broke UI.

[Fryguy]

I wanted to give a status on the automation I've written so far. So, I've put together a simple repo here. For now, the only use case I'm trying to solve is in a core PR someone wants to run the ui specs.

So, the way it will work is

1) In the core PR, someone comments with @miq-bot run_tests manageiq-ui-classic
2) The bot sees that and opens a PR to the new repo, modifying the .travis.yml. Here's an example PR that the bot could create - https://github.com/Fryguy/manageiq-cross_repo/pull/1
3) When it creates the PR, it can backreference the caller, so that github creates a link between the 2 PRs.

So, with the repo in place, half of this is already done, and now I'm just working on the bot support, but it turns out we've never done writing to git before with the bot and it's taking a little longer than anticipated.

Note that right now, even without the bot support, someone could do steps 2 and 3 manually, and it will work (The sample PR above is me doing just that), so we can actually start using this right now (or at least when I transfer it to the ManageIQ org). I've also tested that this script can run any plugin's tests, so we can even do api or providers stuff as well.

However, the bot support is what I'd consider the smallest "real" MVP. Later the bot can monitor the status of the actual run and then use the status API, or perhaps reject the original PR and only approve when it goes green. There are lots of directions for follow up. There are also a ton of other combinations we could do such as running provider tests from a core PR, running the core tests from a ui-classic PR, running the core specs in a core PR in the context of manageiq-schema changes, etc. I have a nice matrix of things we can do.

[himdel]

Is it that common that a non-core repo also breaks the UI? I thought the main issue was that core itself broke UI.

Looking at the data, yeah, never mind :) I do remember suspecting and investigating something in -content or -schema, but looks like it always turned out to be core.
So yeah, we can certainly make things a lot better by just focusing on core :).

(That didn't use to be entirely so before we started tracking, hence the name of the issue, I remember provider repos as a pain point so, maybe the rest will only bite us if there's suddenly a lot of changes in providers :).)

[mzazrivec]

Another issue that might help convince those still not convinced:

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

1) YAML reports regular reports 300_Migration Readiness - Virtual Machines/030_VMware VM Summary defines headers that match col_order
     Failure/Error: expect(headers).to eq(col_order)

       expected: 9
            got: 8

       (compared using ==)
     Shared Example Group: "all report type examples" called from ./spec/product/reports_spec.rb:65
     # ./spec/product/reports_spec.rb:26:in `block (3 levels) in <top (required)>'
  2) YAML reports regular reports 310_Migration Readiness - Providers/010_VMware Environment Summary defines headers that match col_order
     Failure/Error: expect(headers).to eq(col_order)

       expected: 8
            got: 7

       (compared using ==)
     Shared Example Group: "all report type examples" called from ./spec/product/reports_spec.rb:65
     # ./spec/product/reports_spec.rb:26:in `block (3 levels) in <top (required)>'

Cause: https://github.com/ManageIQ/manageiq/pull/18944
Fix: https://github.com/ManageIQ/manageiq/pull/18950

[himdel]

@Fryguy I think the approach you outlined is a nice first step, but I don't expect it to have much effect by itself. It makes it easy to run the tests when you suspect there may be a problem, but... well, if a merger already suspects there may be a problem with a particular PR and doesn't try just running the tests locally, that's just negligence :).

If the goal is to catch unexpected breakages, this does nothing, except perhaps serve as a basis for a policy of "do this for every PR where X,Y,Z".

So.. what are the plans after that?

[bdunne]

It feels like the test mentioned in https://github.com/ManageIQ/manageiq-ui-classic/issues/4921#issuecomment-509581498 is in the wrong repo and should live with the YAML files that it's testing.

[himdel]

Aah, yes.. it was moved together with product/views in https://github.com/ManageIQ/manageiq-ui-classic/pull/1384 / https://github.com/ManageIQ/manageiq/pull/15159, but it doesn't actually test product/views. However, it also tests timelines, charts etc,. all of which should live in the UI.

So.. maybe we need to fix the UI spec to look at product/views instead of product/reports and copy the relevant bit of the spec to core, to check product/reports there?

[himdel]

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

  1) TreeNode::ConfiguredSystem  #tooltip returns the prefixed text
     Failure/Error: expect(subject.tooltip).to eq("#{prefix}: #{subject.text}")

       expected: "Configured System: Configured_system_0000000000003"
            got: "Host Name"

       (compared using ==)
     Shared Example Group: "TreeNode::Node#tooltip prefix" called from ./spec/presenters/tree_node/configured_system_spec.rb:15
     # ./spec/shared/presenters/tree_node/common.rb:36:in `block (3 levels) in <top (required)>'

https://github.com/ManageIQ/manageiq/pull/18977#issuecomment-514261114

Cause: ManageIQ/manageiq#18977
Fix: https://github.com/ManageIQ/manageiq/pull/19045

[mzazrivec]

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

ApplicationHelper#role_allows? permission store consults the permission store
     Failure/Error: expect(Menu::DefaultMenu.services_menu_section.visible?).to be_truthy

       expected: truthy value
            got: false
     # ./spec/helpers/application_helper_spec.rb:62:in `block (5 levels) in <top (required)>'
     # ./spec/helpers/application_helper_spec.rb:50:in `block (4 levels) in <top (required)>'

Cause: https://github.com/ManageIQ/manageiq/pull/19053
Fix: https://github.com/ManageIQ/manageiq-ui-classic/pull/5870

[mzazrivec]

Failing CI: https://travis-ci.org/ManageIQ/manageiq-ui-classic/jobs/608753209?utm_medium=notification&utm_source=github_status

irb(main):002:0> MiqExpression.miq_adv_search_lists('CloudNetwork', :exp_available_finds)
PostgreSQLAdapter#log_after_checkout, connection_pool: size: 30, connections: 1, in use: 1, waiting_in_queue: 0
Traceback (most recent call last):
       16: from railties (5.1.7) lib/rails/commands/console/console_command.rb:97:in `perform'
       15: from railties (5.1.7) lib/rails/commands/console/console_command.rb:17:in `start'
       14: from railties (5.1.7) lib/rails/commands/console/console_command.rb:62:in `start'
       13: from (irb):2
       12: from lib/miq_expression.rb:903:in `miq_adv_search_lists'
       11: from lib/miq_expression.rb:802:in `model_details'
       10: from lib/miq_expression.rb:891:in `get_relats'
        9: from lib/miq_expression.rb:942:in `build_relats'
        8: from lib/miq_expression.rb:942:in `each'
        7: from lib/miq_expression.rb:973:in `block in build_relats'
        6: from lib/miq_expression.rb:942:in `build_relats'
        5: from lib/miq_expression.rb:942:in `each'
        4: from lib/miq_expression.rb:952:in `block in build_relats'
        3: from activerecord (5.1.7) lib/active_record/reflection.rb:404:in `klass'
        2: from activerecord (5.1.7) lib/active_record/reflection.rb:408:in `compute_class'
        1: from activerecord (5.1.7) lib/active_record/inheritance.rb:166:in `compute_type'
NameError (uninitialized constant ManageIQ::Providers::NetworkManager::AuthKeyPair)

Cause: https://github.com/ManageIQ/manageiq/pull/19473#issuecomment-551427468
Fix: https://github.com/ManageIQ/manageiq/pull/19486

[himdel]

https://travis-ci.org/ManageIQ/manageiq-ui-classic/jobs/611907845#L2080

  1) ReportController::Dashboards #db_copy when save button saves unsuccessfully and returns error message
     Failure/Error: expect(JSON.parse(response.body)["error"]["message"]).to include("Error")

     NoMethodError:
       undefined method `[]' for nil:NilClass
     # ./spec/controllers/miq_report_controller/dashboards_spec.rb:29:in `block (5 levels) in <top (required)>'

(the expected response.body is "{\"error\":{\"message\":\"Error during 'Validate': Validation failed: MiqWidgetSet: Name has already been taken\"}}")

Cause: ManageIQ/manageiq#19491
Fix: https://github.com/ManageIQ/manageiq-ui-classic/pull/6416

[himdel]

https://travis-ci.org/ManageIQ/manageiq-ui-classic/jobs/614038504#L2079

  1) VmInfraController the reconfigure tab displays the submit and cancel buttons
     Failure/Error: expect(response.body).to include("miq-button alt='Submit'")
       expected "{\"explorer\":\"flash\",\"replacePartials\":{\"flash_msg_div\":\"\\u003cdiv id='flash_msg_div' style...ong\\u003e\\n\\u003c/div\\u003e\\n\\u003c/div\\u003e\\n\\u003c/div\\u003e\\n\"},\"scrollTop\":true}" to include "miq-button alt='Submit'"
     # ./spec/controllers/vm_infra_controller_spec.rb:566:in `block (2 levels) in <top (required)>'
  2) VmInfraController the reconfigure tab for a vm with max_cpu_cores_per_socket > 1 should display the cpu_cores_per_socket dropdown
     Failure/Error: expect(response.body).to include('Total Processors')
       expected "{\"explorer\":\"flash\",\"replacePartials\":{\"flash_msg_div\":\"\\u003cdiv id='flash_msg_div' style...ong\\u003e\\n\\u003c/div\\u003e\\n\\u003c/div\\u003e\\n\\u003c/div\\u003e\\n\"},\"scrollTop\":true}" to include "Total Processors"
     # ./spec/controllers/vm_infra_controller_spec.rb:532:in `block (2 levels) in <top (required)>'
  3) VmInfraController the reconfigure tab for a single vmware vm should display the list of disks
     Failure/Error: expect(response.body).to include('Disks')
       expected "{\"explorer\":\"flash\",\"replacePartials\":{\"flash_msg_div\":\"\\u003cdiv id='flash_msg_div' style...ong\\u003e\\n\\u003c/div\\u003e\\n\\u003c/div\\u003e\\n\\u003c/div\\u003e\\n\"},\"scrollTop\":true}" to include "Disks"
     # ./spec/controllers/vm_infra_controller_spec.rb:549:in `block (2 levels) in <top (required)>'
  4) ApplicationHelper::Button::VmReconfigure#visible? record is vmware vm behaves like will not be skipped for this record is expected to be truthy
     Failure/Error: expect(button.visible?).to be_truthy

       expected: truthy value
            got: false
     Shared Example Group: "will not be skipped for this record" called from ./spec/helpers/application_helper/buttons/vm_reconfigure_spec.rb:8
     # ./spec/shared/helpers/application_helper_buttons.rb:13:in `block (2 levels) in <top (required)>'
  5) ApplicationController Certain actions should be allowed only for a VM record
     Failure/Error: expect(controller.send(:flash_errors?)).not_to be_truthy

       expected: falsey value
            got: true
     # ./spec/controllers/application_controller/ci_processing_spec.rb:621:in `block (3 levels) in <top (required)>'
     # ./spec/controllers/application_controller/ci_processing_spec.rb:618:in `each'
     # ./spec/controllers/application_controller/ci_processing_spec.rb:618:in `block (2 levels) in <top (required)>'

Cause: https://github.com/ManageIQ/manageiq-providers-ovirt/pull/421, https://github.com/ManageIQ/manageiq-providers-vmware/pull/467

Fix: https://github.com/ManageIQ/manageiq-ui-classic/pull/6428

[agrare]

In this last case it was a provider change that caused the failure.

I don't think that running the UI specs on every core PR _and_ every provider PR is going to be very reasonable.

[himdel]

Well, @agrare how this should work is that we have a set of provider methods that are guaranteed to have a stable interface, and test that UI and everything else just use those methods. Then there would be no need for this, you could just test those methods in provider repos. (IIRC that was even an explicit target at some point, we can't really have pluggable providers without it.)

But, until that happens, the UI (and API) is the main place where provider code and core code come together. We are the integration. So, I think it's absolutely reasonable to run the UI tests everywhere.

Of course, if we do that, we may want to go through those tests and comb them so that tests that won't catch any of these problems only run ui side. But, that's probably just JS tests now anyway. Really.

---

I mean, we don't have to run tests that test our toolbar implementation in providers.
But we absolutely have to run tests that evaulate individual toolbar definitions, as long as toolbars need to call provider methods to determine whether they should be enabled or not.
If those provider methods break, the whole app breaks, so it absolutely makes sense to tests that, even though those are just "ui toolbar tests" :).

That, or write actual shared provider tests that test that every method the UI needs keeps the same interface :).

[martinpovolny]

cc: @chessbyte

[agrare]

how this should work is that we have a set of provider methods that are guaranteed to have a stable interface, and test that UI and everything else just use those methods. Then there would be no need for this, you could just test those methods in provider repos

We completely agree on this point, lets add moving provider specifics in UI specs to the pluggability checklist https://github.com/ManageIQ/manageiq/issues/19440 so we can start moving the provider logic out of the UI.

[himdel]

https://travis-ci.org/ManageIQ/manageiq-ui-classic/jobs/615790473#L2060

  1) Mixins::Actions::VmActions::Rename#rename_save getting proper task_id initiates wait for task
     Failure/Error: expect(controller).to receive(:initiate_wait_for_task).with(:task_id => kind_of(Integer), :action => 'rename_finished')
     ...@flash_array=[{:message=>{:text=>"VM rename: Task start failed",...

Cause: https://github.com/ManageIQ/manageiq/pull/19544
Fix: https://github.com/ManageIQ/manageiq/pull/19553

[martinpovolny]

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

1) CloudVolumeController#create_volume in Amazon EBS for volume type 'gp2' behaves like queue create volume task queues the create cloud volume action form OpenStack
     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 Volume for user user9", :userid=>"user9"}, {:args=>[52000000000033, {:availability_zone=>"us-east-1e", :encrypted=>nil, :name=>"volume", :size=>...class_name=>"CloudVolume", :method_name=>"create_volume", :role=>"ems_operations", :zone=>"Zone 22"})
              got: ({:action=>"creating Cloud Volume for user user9", :userid=>"user9"}, {:args=>[52000000000033, {:availability_zone=>"us-east-1e", :encrypted=>nil, :name=>"volume", :size=>...Volume", :method_name=>"create_volume", :queue_name=>nil, :role=>"ems_operations", :zone=>"Zone 22"})
       Diff:
       @@ -8,6 +8,7 @@
             :volume_type=>"gp2"}],
          :class_name=>"CloudVolume",
          :method_name=>"create_volume",
       +  :queue_name=>nil,
          :role=>"ems_operations",
          :zone=>"Zone 22"}]

Cause: https://github.com/ManageIQ/manageiq/pull/19570
Fix: https://github.com/ManageIQ/manageiq-ui-classic/pull/6482

[himdel]

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

rspec ./spec/controllers/cloud_object_store_container_spec.rb:165 # CloudObjectStoreContainerController create object store container in Amazon S3 create behaves like queue create container task queues the create cloud object store container action form
rspec ./spec/controllers/cloud_tenant_controller_spec.rb:175 # CloudTenantController#delete queues the delete action
rspec ./spec/controllers/cloud_tenant_controller_spec.rb:149 # CloudTenantController#edit queues the update action
rspec ./spec/controllers/cloud_tenant_controller_spec.rb:117 # CloudTenantController#create queues the create action

(all the specs fail on the same thing:)

     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 Object Store Container for user user3219", :userid=>"user3219"}, {:args=>[34000000001988, {:create_bucket_configuration=>{:location_constraint=>"eu-central-1"}, :name...", :method_name=>"cloud_object_store_container_create", :role=>"ems_operations", :zone=>"Zone 3052"})
              got: ({:action=>"creating Cloud Object Store Container for user user3219", :userid=>"user3219"}, {:args=>[34000000001988, {:create_bucket_configuration=>{:location_constraint=>"eu-central-1"}, :name...cloud_object_store_container_create", :queue_name=>nil, :role=>"ems_operations", :zone=>"Zone 3052"})
       Diff:
       @@ -6,6 +6,7 @@
             :name=>"bucket-01"}],
          :class_name=>"CloudObjectStoreContainer",
          :method_name=>"cloud_object_store_container_create",
       +  :queue_name=>nil,
          :role=>"ems_operations",
          :zone=>"Zone 3052"}]

Cause: https://github.com/ManageIQ/manageiq/pull/19602, https://github.com/ManageIQ/manageiq/pull/19603
Fix: https://github.com/ManageIQ/manageiq-ui-classic/pull/6503

[agrare]

Created https://github.com/ManageIQ/manageiq-ui-classic/issues/6504 to track decoupling of UI tests from core operations methods

[himdel]

https://github.com/ManageIQ/manageiq/pull/19754
https://github.com/ManageIQ/manageiq-ui-classic/pull/6662

rspec ./spec/helpers/application_helper/buttons/storage_scan_spec.rb:35 # ApplicationHelper::Button::StorageScan when feature is supported #calculate_properties with valid credentials for this Datastore behaves like an enabled button is expected to be truthy
rspec ./spec/helpers/application_helper/buttons/storage_scan_spec.rb:32 # ApplicationHelper::Button::StorageScan when feature is supported #calculate_properties but no EMS has valid credentials for the Datastore behaves like a disabled button is expected to eq "There are no EMSs with valid credentials for this Datastore"

[skateman]

Upgrading kubeclient broke stuff in the UI:

1) EmsContainerController.update_ems_button_detect with route set detects openshift prometheus alert route
     Failure/Error: expect(mock_client).to receive(:discover)

       (Double "kubeclient").discover(*(any args))
           expected: 1 time with any arguments
           received: 2 times with any arguments
     # ./spec/controllers/ems_container_controller_spec.rb:74:in `block (4 levels) in <top (required)>'
  2) EmsContainerController.update_ems_button_detect with route set tolerates detection exceptions
     Failure/Error: expect(mock_client).to receive(:discover)

       (Double "kubeclient").discover(*(any args))
           expected: 1 time with any arguments
           received: 2 times with any arguments
     # ./spec/controllers/ems_container_controller_spec.rb:74:in `block (4 levels) in <top (required)>'
  3) EmsContainerController.update_ems_button_detect with route set detects openshift prometheus metric route
     Failure/Error: expect(mock_client).to receive(:discover)

       (Double "kubeclient").discover(*(any args))
           expected: 1 time with any arguments
           received: 2 times with any arguments
     # ./spec/controllers/ems_container_controller_spec.rb:74:in `block (4 levels) in <top (required)>'
  4) EmsContainerController.update_ems_button_detect with route set detects openshift hawkular metric route
     Failure/Error: expect(mock_client).to receive(:discover)

       (Double "kubeclient").discover(*(any args))
           expected: 1 time with any arguments
           received: 2 times with any arguments
     # ./spec/controllers/ems_container_controller_spec.rb:74:in `block (4 levels) in <top (required)>'

https://github.com/ManageIQ/manageiq-ui-classic/pull/7051

[agrare]

Hey @skateman I suspect this wasn't upgrading kubeclient or https://github.com/ManageIQ/manageiq-ui-classic/pull/7043 (since cross-repo tests were green on that one). I'm guessing it was probably https://github.com/ManageIQ/manageiq-providers-openshift/pull/174 which changed how the method was internally constructed even though the API stayed the same. The test should probably be rewritten to not expect anything from the underlying kubeclient connection.

[skateman]

Hey @skateman I suspect this wasn't upgrading kubeclient or #7043 (since cross-repo tests were green on that one). I'm guessing it was probably ManageIQ/manageiq-providers-openshift#174 which changed how the method was internally constructed even though the API stayed the same. The test should probably be rewritten to not expect anything from the underlying kubeclient connection.

@mzazrivec can you create a new spec according to this? I guess we need allow instead of expect.

[mzazrivec]

Cause: https://github.com/ManageIQ/manageiq/pull/20361
Fix: https://github.com/ManageIQ/manageiq-ui-classic/pull/7214

[gtanzillo]

Closing since we now have cross repo tests