-
Notifications
You must be signed in to change notification settings - Fork 3.9k
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: hard code date values in Date_column_types_validation_spec #36759
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -43,17 +43,16 @@ describe( | |
// Click unix cell edit | ||
table.ClickOnEditIcon(row, column); | ||
|
||
// Click on specific date within | ||
// Click on a specific date within the view port of the date picker | ||
// Date picker opens in september 2024 due to the Table data set | ||
agHelper.GetNClick( | ||
`${table._dateInputPopover} [aria-label='${table.getFormattedTomorrowDates().verboseFormat}']`, | ||
`${table._dateInputPopover} [aria-label='Thu Sep 26 2024']`, | ||
); | ||
|
||
// Check that date is set in column | ||
// Check that the date is set in column | ||
table | ||
.ReadTableRowColumnData(row, column, "v2") | ||
.then((val) => | ||
expect(val).to.equal(table.getFormattedTomorrowDates().isoFormat), | ||
); | ||
.then((val) => expect(val).to.equal("2024-09-26")); | ||
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. Class, let's discuss this assertion change. The assertion now uses a hardcoded date value "2024-09-26" instead of a dynamic method. While this matches the clicked date, it doesn't align with our goal of working with relative dates. Let's think about how we can make this assertion more flexible. Could we use a variable that represents 'tomorrow' or 'today + 1 day' instead of a fixed date? This would make our test more robust and less likely to fail due to date changes. 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. Do we think this is also hard coded? 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. Yes, this is a hardcoded date value, i.e when the fixed date from the Table data is used, this hardcoded date here is always visible in the date picker viewport and will be selected. |
||
}; | ||
|
||
it("1. should allow inline editing of Unix Timestamp in seconds (unix/s)", () => { | ||
|
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.
Attention, students! We need to improve our selector usage.
While the change addresses the issue with the fixed date, it doesn't fully solve our problem. We're still using a hardcoded date, which goes against our goal of handling relative dates.
Let's think about how we can make this more dynamic. Can you come up with a way to select the date based on 'today' or 'tomorrow' instead of a fixed date? Remember, we want to avoid hardcoding specific dates in our tests.