search

etalab / transport-site

Rendre disponible, valoriser et améliorer les données transports
https://transport.data.gouv.fr
191 stars 30 forks source link

3 tests dans `apps/transport` sont trop couplés aux métriques #3975

Open thbar opened 4 months ago

thbar commented 4 months ago

Ticket pour prévenir un "WTF" pour celui qui tombera dessus (c'est initialement très confusant), sans urgence immédiate à traiter, mais à améliorer dans le temps.

En travaillant sur:

et en particulier en débuggant la télémétrie, je suis tombé sur l'os suivant: si un test de apps/unlock génère un évènement de télémétrie lié aux métriques à un moment où les handlers ne sont pas déconnectés d'Ecto (handlers qui ne sont pas connus de apps/unlock mais sont activés au boot général), un métrique sera inséré en base (comme attendu), mais ce métrique génèrera alors des failures (3 répertoriées actuellement) liées à un couplage trop fort de tests de apps/transport sur le contenu de la base métriques.

Ce couplage trop fort nous posera d'autres problèmes, et pourrait être réglé avec un pattern du type assert_difference (voir https://apidock.com/rails/v7.1.3.2/ActiveSupport/Testing/Assertions/assert_difference) plutôt qu'une comparaison brute par égalité.

Je vois 3 failures pour le moment:

``` ❯ mix test 08:56:11.203 [info] Migrations already up ==> shared ............................................................................. Finished in 1.0 seconds (0.7s async, 0.3s sync) 23 doctests, 54 tests, 0 failures Randomized with seed 433334 ==> datagouvfr ................ Finished in 0.3 seconds (0.2s async, 0.07s sync) 16 tests, 0 failures Randomized with seed 433334 ==> unlock Excluding tags: [:pending] ..**....................................*..... Finished in 0.9 seconds (0.6s async, 0.2s sync) 6 doctests, 40 tests, 0 failures, 3 skipped Randomized with seed 433334 ==> gbfs Excluding tags: [:pending] .......................... Finished in 0.7 seconds (0.5s async, 0.2s sync) 27 tests, 0 failures, 1 excluded Randomized with seed 433334 ==> transport Excluding tags: [:pending, :transport_tools, :documentation_links] ................... 1) test archives rows for a given day (Transport.Test.Transport.Jobs.ArchiveMetricsJobTest) apps/transport/test/transport/jobs/archive_metrics_job_test.exs:50 match (=) failed The following variables were pinned: test_date = ~U[2024-03-05 00:00:00Z] day_before = ~U[2024-03-04 00:00:00Z] day_after = ~U[2024-03-06 00:00:00Z] code: assert [ %DB.Metrics{target: "foo", event: "baz", period: ^day_before, count: 10}, %DB.Metrics{target: "foo", event: "baz", period: ^test_date, count: 3}, %DB.Metrics{target: "foo", event: "bar", period: ^test_date, count: 4}, %DB.Metrics{target: "foo", event: "baz", period: ^day_after, count: 5} ] = metrics left: [ %DB.Metrics{ count: 10, event: "baz", period: ^day_before, target: "foo" }, %DB.Metrics{ count: 3, event: "baz", period: ^test_date, target: "foo" }, %DB.Metrics{ count: 4, event: "bar", period: ^test_date, target: "foo" }, %DB.Metrics{ count: 5, event: "baz", period: ^day_after, target: "foo" } ] right: [ %DB.Metrics{ count: 10, event: "baz", period: ~U[2024-03-04 00:00:00Z], target: "foo", __meta__: #Ecto.Schema.Metadata<:loaded, "metrics">, id: nil, inserted_at: nil, updated_at: nil }, %DB.Metrics{ count: 3, event: "baz", period: ~U[2024-03-05 00:00:00Z], target: "foo", __meta__: #Ecto.Schema.Metadata<:loaded, "metrics">, id: nil, inserted_at: nil, updated_at: nil }, %DB.Metrics{ count: 4, event: "bar", period: ~U[2024-03-05 00:00:00Z], target: "foo", __meta__: #Ecto.Schema.Metadata<:loaded, "metrics">, id: nil, inserted_at: nil, updated_at: nil }, %DB.Metrics{ count: 5, event: "baz", period: ~U[2024-03-06 00:00:00Z], target: "foo", __meta__: #Ecto.Schema.Metadata<:loaded, "metrics">, id: nil, inserted_at: nil, updated_at: nil }, %DB.Metrics{__meta__: #Ecto.Schema.Metadata<:loaded, "metrics">, id: nil, target: "proxy:foo", event: "proxy:request:internal", period: ~U[2024-06-04 06:00:00Z], count: 1, inserted_at: nil, updated_at: nil}, %DB.Metrics{__meta__: #Ecto.Schema.Metadata<:loaded, "metrics">, id: nil, target: "proxy:foo", event: "proxy:request:external", period: ~U[2024-06-04 06:00:00Z], count: 1, inserted_at: nil, updated_at: nil}, %DB.Metrics{__meta__: #Ecto.Schema.Metadata<:loaded, "metrics">, id: nil, target: "proxy:some-identifier", event: "proxy:request:external", period: ~U[2024-06-04 06:00:00Z], count: 2, inserted_at: nil, updated_at: nil}, %DB.Metrics{__meta__: #Ecto.Schema.Metadata<:loaded, "metrics">, id: nil, target: "proxy:some-identifier", event: "proxy:request:internal", period: ~U[2024-06-04 06:00:00Z], count: 2, inserted_at: nil, updated_at: nil}, %DB.Metrics{__meta__: #Ecto.Schema.Metadata<:loaded, "metrics">, id: nil, target: "proxy:an-existing-identifier", event: "proxy:request:internal", period: ~U[2024-06-04 06:00:00Z], count: 2, inserted_at: nil, updated_at: nil}, %DB.Metrics{__meta__: #Ecto.Schema.Metadata<:loaded, "metrics">, id: nil, target: "proxy:an-existing-identifier", event: "proxy:request:external", period: ~U[2024-06-04 06:00:00Z], count: 3, inserted_at: nil, updated_at: nil}, %DB.Metrics{__meta__: #Ecto.Schema.Metadata<:loaded, "metrics">, id: nil, target: "proxy:an-existing-aggregate-identifier:first-remote", event: "proxy:request:internal", period: ~U[2024-06-04 06:00:00Z], count: 8, inserted_at: nil, updated_at: nil}, %DB.Metrics{__meta__: #Ecto.Schema.Metadata<:loaded, "metrics">, id: nil, target: "proxy:an-existing-aggregate-identifier:second-remote", event: "proxy:request:internal", period: ~U[2024-06-04 06:00:00Z], count: 8, inserted_at: nil, updated_at: nil}, %DB.Metrics{__meta__: #Ecto.Schema.Metadata<:loaded, "metrics">, id: nil, target: "proxy:an-existing-aggregate-identifier", event: "proxy:request:external", period: ~U[2024-06-04 06:00:00Z], count: 9, inserted_at: nil, updated_at: nil} ] stacktrace: test/transport/jobs/archive_metrics_job_test.exs:77: (test) ............................................................. 2) test renders the expected counts (Transport.TransportWeb.ProxyRequestsCountLiveTest) apps/transport/test/transport_web/live_views/proxy_requests_count_live_test.exs:11 match (=) failed code: assert %{socket: %Phoenix.LiveView.Socket{assigns: %{data: %{"external" => 5, "internal" => 3}}}} = :sys.get_state(view.pid) left: %{ socket: #Phoenix.LiveView.Socket 5, "internal" => 3}}, transport_pid: nil, ...> } right: %{ socket: #Phoenix.LiveView.Socket, router: nil, assigns: %{__changed__: %{}, data: %{"external" => 20, "internal" => 24}, doc_url: "https://doc.transport.data.gouv.fr/type-donnees/operateurs-de-transport-regulier-de-personnes/administration-des-donnees-transport-public-collectif/publier-des-donnees-temps-reel/serveur-proxy-gtfs-rt", flash: %{}, live_action: nil}, transport_pid: #PID<0.2526.0>, ...>, components: {%{}, %{}, 1}, join_ref: 0, serializer: Phoenix.LiveViewTest.ClientProxy, topic: "lv:phx-F9W6wyOA1kX1RRbh", upload_names: %{}, upload_pids: %{} } stacktrace: test/transport_web/live_views/proxy_requests_count_live_test.exs:25: (test) ............................................................................................................................................................................................. 3) test get with an existing conversion (TransportWeb.ConversionControllerTest) apps/transport/test/transport_web/controllers/conversion_controller_test.exs:32 match (=) failed The following variables were pinned: target = "resource_id:764" period = ~U[2024-06-04 00:00:00Z] code: assert [%DB.Metrics{target: ^target, event: "conversions:get:GeoJSON", period: ^period, count: 1}] = DB.Metrics |> DB.Repo.all() left: [ %DB.Metrics{ count: 1, event: "conversions:get:GeoJSON", period: ^period, target: ^target } ] right: [ %DB.Metrics{ count: 1, event: "conversions:get:GeoJSON", period: ~U[2024-06-04 00:00:00Z], target: "resource_id:764", __meta__: #Ecto.Schema.Metadata<:loaded, "metrics">, id: 143, inserted_at: ~U[2024-06-04 06:56:29.980651Z], updated_at: ~U[2024-06-04 06:56:29.980651Z] }, %DB.Metrics{__meta__: #Ecto.Schema.Metadata<:loaded, "metrics">, id: 97, target: "proxy:foo", event: "proxy:request:internal", period: ~U[2024-06-04 06:00:00Z], count: 1, inserted_at: ~U[2024-06-04 06:56:14.799162Z], updated_at: ~U[2024-06-04 06:56:14.799162Z]}, %DB.Metrics{__meta__: #Ecto.Schema.Metadata<:loaded, "metrics">, id: 98, target: "proxy:foo", event: "proxy:request:external", period: ~U[2024-06-04 06:00:00Z], count: 1, inserted_at: ~U[2024-06-04 06:56:14.799105Z], updated_at: ~U[2024-06-04 06:56:14.799105Z]}, %DB.Metrics{__meta__: #Ecto.Schema.Metadata<:loaded, "metrics">, id: 95, target: "proxy:an-existing-identifier", event: "proxy:request:internal", period: ~U[2024-06-04 06:00:00Z], count: 2, inserted_at: ~U[2024-06-04 06:56:14.797903Z], updated_at: ~U[2024-06-04 06:56:14.797903Z]}, %DB.Metrics{__meta__: #Ecto.Schema.Metadata<:loaded, "metrics">, id: 96, target: "proxy:an-existing-identifier", event: "proxy:request:external", period: ~U[2024-06-04 06:00:00Z], count: 3, inserted_at: ~U[2024-06-04 06:56:14.797825Z], updated_at: ~U[2024-06-04 06:56:14.797825Z]}, %DB.Metrics{__meta__: #Ecto.Schema.Metadata<:loaded, "metrics">, id: 73, target: "proxy:some-identifier", event: "proxy:request:internal", period: ~U[2024-06-04 06:00:00Z], count: 2, inserted_at: ~U[2024-06-04 06:56:14.661158Z], updated_at: ~U[2024-06-04 06:56:14.661158Z]}, %DB.Metrics{__meta__: #Ecto.Schema.Metadata<:loaded, "metrics">, id: 75, target: "proxy:some-identifier", event: "proxy:request:external", period: ~U[2024-06-04 06:00:00Z], count: 2, inserted_at: ~U[2024-06-04 06:56:14.661019Z], updated_at: ~U[2024-06-04 06:56:14.661019Z]}, %DB.Metrics{__meta__: #Ecto.Schema.Metadata<:loaded, "metrics">, id: 72, target: "proxy:an-existing-aggregate-identifier", event: "proxy:request:external", period: ~U[2024-06-04 06:00:00Z], count: 9, inserted_at: ~U[2024-06-04 06:56:14.660964Z], updated_at: ~U[2024-06-04 06:56:14.660964Z]}, %DB.Metrics{__meta__: #Ecto.Schema.Metadata<:loaded, "metrics">, id: 77, target: "proxy:an-existing-aggregate-identifier:first-remote", event: "proxy:request:internal", period: ~U[2024-06-04 06:00:00Z], count: 8, inserted_at: ~U[2024-06-04 06:56:14.661003Z], updated_at: ~U[2024-06-04 06:56:14.661003Z]}, %DB.Metrics{__meta__: #Ecto.Schema.Metadata<:loaded, "metrics">, id: 71, target: "proxy:an-existing-aggregate-identifier:second-remote", event: "proxy:request:internal", period: ~U[2024-06-04 06:00:00Z], count: 8, inserted_at: ~U[2024-06-04 06:56:14.660992Z], updated_at: ~U[2024-06-04 06:56:14.660992Z]} ] stacktrace: test/transport_web/controllers/conversion_controller_test.exs:74: (test) ......................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................... Finished in 55.7 seconds (27.8s async, 27.9s sync) 189 doctests, 978 tests, 3 failures, 6 excluded Randomized with seed 433334 ```

Pour reproduire:

thbar commented 4 months ago

Stratégies possibles pour mitiger, pour mémoire: