Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

test: provide timing information #779

Merged
merged 7 commits into from
Jul 20, 2023
Merged

test: provide timing information #779

merged 7 commits into from
Jul 20, 2023

Conversation

vemv
Copy link
Member

@vemv vemv commented Jul 18, 2023

Fixes #755

With these changes, responses are now enhanced with the following structural changes:

+{:elapsed-time {:humanized "Completed in 1028 ms", :ms 1028},
+ :ns-elapsed-time
+ {:failing-test-ns {:humanized "Completed in 1026 ms", :ms 1026}},
 :summary {:error 0, :fail 2, :ns 1, :pass 0, :test 2, :var 2},
 :testing-ns "failing-test-ns",
 :gen-input [],
 :status #{"done"},
 :id "52816a02-86cc-457a-a9fa-b287e90da189",
 :session #{"e3b12907-d64e-4e6d-902a-ab63fef9c811"},
 :results
 {:failing-test-ns
  {:fast-failing-test
   [{:index 0,
     :ns "failing-test-ns",
     :file "failing_test_ns.clj",
     :type "fail",
     :diffs [["1\n" ["0\n" "1\n"]]],
     :line 5,
     :var "fast-failing-test",
     :expected "0\n",
     :context [],
+    :elapsed-time {:humanized "Completed in 20 ms", :ms 20},
     :actual "1\n",
     :message ""}],
   :slow-failing-test
   [{:index 0,
     :ns "failing-test-ns",
     :file "failing_test_ns.clj",
     :type "fail",
     :diffs [["1\n" ["0\n" "1\n"]]],
     :line 9,
     :var "slow-failing-test",
     :expected "0\n",
     :context [],
+    :elapsed-time {:humanized "Completed in 1006 ms", :ms 1006},
     :actual "1\n",
     :message ""}]}}}

Cheers - V

@vemv vemv requested a review from bbatsov July 18, 2023 11:26
@vemv vemv changed the title test: provide testing information test: provide timing information Jul 18, 2023
~@body)
took# (- (System/currentTimeMillis)
then#)]
(reset! ~time-atom {:took-ms took#
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd probably use names like :elapsed-time and :elapsed-time-string/humanized, as I find the use of took confusing. Other terms that come to mind duration, time, etc. I recall prepl just uses the :ms key, so I can live with something like this as well.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As this is already under :timing you can also simply go with :ms and :humanized. Perhaps :timing can become :elapsed-time or :duration?

~@body)
took# (- (System/currentTimeMillis)
then#)]
(reset! ~time-atom {:took-ms took#
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd probably use names like :elapsed-time and :elapsed-time-string/humanized, as I find the use of took confusing. Other terms that come to mind duration, time, etc. I recall prepl just uses the :ms key, so I can live with something like this as well.

@bbatsov
Copy link
Member

bbatsov commented Jul 18, 2023

Other than my remark about the naming - everything looks good.

@vemv
Copy link
Member Author

vemv commented Jul 18, 2023

All set!

I also updated the PR desc with the new names and the newly implemented global key.

@@ -1278,3 +1292,310 @@ Optional parameters::
Returns::
* `:status` done



=== `log-add-appender`
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I ran lein docs so these showed up.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Guess Roman forgot to run this.

@bbatsov bbatsov merged commit 0aed2e4 into master Jul 20, 2023
@bbatsov bbatsov deleted the 755--timing branch July 20, 2023 05:21
@vemv
Copy link
Member Author

vemv commented Jul 20, 2023

I've cut v0.32.0-alpha2 to play with this cider.el side

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Feature idea: Timing information in test report
2 participants