-
Notifications
You must be signed in to change notification settings - Fork 29k
[SPARK-5931][CORE] Use consistent naming for time properties #5236
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
Changes from 1 commit
59bf9e1
404f8c3
7db6d2a
4933fda
c9f5cad
21ef3dd
064ebd6
7320c87
272c215
3352d34
3f1cfc8
6d1518e
2fcc91c
5181597
c6a0095
cde9bff
42477aa
9a29d8d
34f87c2
8f741e1
9e2547c
499bdf0
5232a36
3a12dd8
68f4e93
70ac213
647b5ac
1c0c07c
8613631
bac9edf
1858197
3b126e1
39164f9
b2fc965
dd0a680
bf779b0
76cfa27
5193d5f
6387772
19c31af
ff40bfe
28187bf
1465390
cbf41db
d4efd26
4e48679
1a1122c
cbd2ca6
6f651a8
7d19cdd
dc7bd08
69fedcc
8927e66
642a06d
25d3f52
bc04e05
951ca2d
f5fafcd
de3bff9
4526c81
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
- Loading branch information
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -39,58 +39,44 @@ class UtilsSuite extends FunSuite with ResetSystemProperties { | |
|
|
||
| test("timeConversion") { | ||
| // Test -1 | ||
| assert(Utils.timeStringAsSec("-1") === -1) | ||
| assert(Utils.timeStringAsSeconds("-1") === -1) | ||
|
|
||
| // Test zero | ||
| assert(Utils.timeStringAsSec("0") === 0) | ||
| assert(Utils.timeStringAsSeconds("0") === 0) | ||
|
|
||
| assert(Utils.timeStringAsSec("1") === 1) | ||
| assert(Utils.timeStringAsSec("1s") === 1) | ||
| assert(Utils.timeStringAsSec("1000ms") === 1) | ||
| assert(Utils.timeStringAsSec("1000000us") === 1) | ||
| assert(Utils.timeStringAsSec("1min") === TimeUnit.MINUTES.toSeconds(1)) | ||
| assert(Utils.timeStringAsSec("1h") === TimeUnit.HOURS.toSeconds(1)) | ||
| assert(Utils.timeStringAsSec("1d") === TimeUnit.DAYS.toSeconds(1)) | ||
| assert(Utils.timeStringAsSeconds("1") === 1) | ||
| assert(Utils.timeStringAsSeconds("1s") === 1) | ||
| assert(Utils.timeStringAsSeconds("1000ms") === 1) | ||
| assert(Utils.timeStringAsSeconds("1000000us") === 1) | ||
| assert(Utils.timeStringAsSeconds("1m") === TimeUnit.MINUTES.toSeconds(1)) | ||
| assert(Utils.timeStringAsSeconds("1min") === TimeUnit.MINUTES.toSeconds(1)) | ||
| assert(Utils.timeStringAsSeconds("1h") === TimeUnit.HOURS.toSeconds(1)) | ||
| assert(Utils.timeStringAsSeconds("1d") === TimeUnit.DAYS.toSeconds(1)) | ||
|
|
||
| assert(Utils.timeStringAsMs("1") === 1) | ||
| assert(Utils.timeStringAsMs("1ms") === 1) | ||
| assert(Utils.timeStringAsMs("1000us") === 1) | ||
| assert(Utils.timeStringAsMs("1s") === TimeUnit.SECONDS.toMillis(1)) | ||
| assert(Utils.timeStringAsMs("1m") === TimeUnit.MINUTES.toMillis(1)) | ||
| assert(Utils.timeStringAsMs("1min") === TimeUnit.MINUTES.toMillis(1)) | ||
| assert(Utils.timeStringAsMs("1h") === TimeUnit.HOURS.toMillis(1)) | ||
| assert(Utils.timeStringAsMs("1d") === TimeUnit.DAYS.toMillis(1)) | ||
|
|
||
| // Test invalid strings | ||
| try { | ||
| intercept[NumberFormatException] { | ||
| Utils.timeStringAsMs("This breaks 600s") | ||
| assert(false) // We should never reach this | ||
| } catch { | ||
| case e: NumberFormatException => assert(true) | ||
| } | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. there's this really cool thing called
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. That is actually super cool. Thanks for pointing that out. |
||
|
|
||
| // Test invalid strings | ||
| try { | ||
| Utils.timeStringAsMs("600ds") | ||
| assert(false) // We should never reach this | ||
| } catch { | ||
| case e: NumberFormatException => assert(true) | ||
| intercept[NumberFormatException] { | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nit: keep this line |
||
| Utils.timeStringAsMs("This breaks 600ds") | ||
| } | ||
|
|
||
|
|
||
| // Test invalid strings | ||
| try { | ||
| intercept[NumberFormatException] { | ||
| Utils.timeStringAsMs("600s This breaks") | ||
| assert(false) // We should never reach this | ||
| } catch { | ||
| case e: NumberFormatException => assert(true) | ||
| } | ||
|
|
||
| // Test invalid strings | ||
| try { | ||
| intercept[NumberFormatException] { | ||
| Utils.timeStringAsMs("This 123s breaks") | ||
| assert(false) // We should never reach this | ||
| } catch { | ||
| case e: NumberFormatException => assert(true) | ||
| } | ||
| } | ||
|
|
||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
just an idea, maybe we can rewrite this as:
Looks nicer IMO, less duplicate code