Skip to content
Snippets Groups Projects
This project is mirrored from https://github.com/metabase/metabase. Pull mirroring updated .
  1. Nov 14, 2023
  2. Nov 13, 2023
  3. Nov 12, 2023
  4. Nov 11, 2023
  5. Nov 10, 2023
    • Mark Bastian's avatar
      Fixing flaky tests related to `inner-query-name-collisions-test` (#32355) (#35612) · efea8d01
      Mark Bastian authored
      It appears the code fails when run in parallel due to the `metabot-settings/enum-cardinality-threshold` value.
      
      To demonstrate, I ran:
      
      ```clojure
      (frequencies
        (pmap
        (fn [_]
          (clojure.test/run-test inner-query-name-collisions-test))
        (range 1000)))
      ```
      
      and got:
      
      ```
      {{:test 1, :pass 12, :fail 0, :error 0, :type :summary} 414, {:test 1, :pass 11, :fail 1, :error 0, :type :summary} 586}
      ```
      
      Running with a simple sequential map produced no errors.
      
      Wrapping the second test in `tu/with-temporary-setting-values [metabot-settings/enum-cardinality-threshold 10]`
      reduced the error count dramatically (in the teens for 1000 samples), but still wasn't bulletproof.
      I'm guessing the two `testing` directives in the test are run in parallel and the setting is not thread safe.
      
      By adding the explicit threshold to the test and breaking it out into a standalone test the
      error count drops dramatically.
      
      ```
      (frequencies
        (pmap
          (fn [_]
            (clojure.test/run-test inner-query-name-collisions-with-joins-test))
          (range 1000)))
      ;=> {{:test 1, :pass 6, :fail 0, :error 0, :type :summary} 998, {:test 1, :pass 5, :fail 1, :error 0, :type :summary} 2}
      ```
      
      I'm still not sure why this fails (only twice in 1000 runs), but apparently something about
      this operation is not thread safe. However, given the additional isolation added by this PR
      we go from a 60ish percent failure rate down to a 0.2% failure rate locally.
      
      Hopefully this is adequate enough to prevent any substantial level of future flakes.
      efea8d01
    • Jerry Huang's avatar
      f2823e4c
    • Ryan Laurie's avatar
      f685f1c1
    • Jeff Bruemmer's avatar
      links (#35603) · 65dc7663
      Jeff Bruemmer authored
      65dc7663
    • John Swanson's avatar
      Audit log CRUD permission failures (#35391) · e825ef2c
      John Swanson authored
      * Audit log CRUD permission failures
      
      When we run `metabase.api.common/*-check` functions like `update-check` or `create-check`, publish an event when the
      check fails, along with enough context that, on the other side, the audit log handler can record the relevant event.
      
      Note that we won't fire events for a `read-check` failure. Eventually we will and these events will be handled by the "read log" instead of the audit log.
      
      If we throw an event that isn't derived from `:metabase/event` it will throw an exception. That `derive` no longer belongs in the
      audit log code, and until we have the view log there's not really a logical place to put it. For now, I just commented
      out the `publish-event!` call, with the assumption that the view log is coming very soon. Otherwise we can just delete
      it and add it back in when it's needed.
      
      Some tangentially related required changes here to make this work include:
      
      * allow `audit-log/model-name` to work with RootCollections
      
      `(t2/model root-collection)` returns a Class rather than a keyword - `name` doesn't work here, so we'll use
      `getSimpleName` instead.
      
      * add a default model-details clause for dashboards
      
      If the event-type is not one that we've explicitly delineated, just return an empty map for the model details.
      
      * Migration to fit longer model names in `activity.model`
      
      The `NativeQuerySnippet` or `PermissionsGroupMembership` models, for example, were too long to fit in `VARCHAR(16)`.
      I'm doubling the length of this column to fit the longest model I see, with a little extra room.
      e825ef2c
    • Uladzimir Havenchyk's avatar
    • Nemanja Glumac's avatar
      Further optimize static-viz check for the backend workflow (#35580) · aa2c4ec1
      Nemanja Glumac authored
      Follow up after #34209
      aa2c4ec1
    • Noah Moss's avatar
      Speed up audit DB tests (#35522) · 6845d6a9
      Noah Moss authored
      
      Co-authored-by: default avatarbryan <bryan.maass@gmail.com>
      6845d6a9
    • Nemanja Glumac's avatar
      Refine default CI triggers (#35617) · 77f0b766
      Nemanja Glumac authored
      77f0b766
    • Ryan Laurie's avatar
      Give cljs tests 15 min (#35608) · e66d80d9
      Ryan Laurie authored
      e66d80d9
Loading