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.

Sign up for GitHub

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

Jump to bottom

Fix future-date test from articles-post file #97

Closed
3 tasks done
Slawomir-DKl opened this issue May 5, 2024 · 5 comments · Fixed by #102
Closed
3 tasks done

Fix future-date test from articles-post file #97

Slawomir-DKl opened this issue May 5, 2024 · 5 comments · Fixed by #102
Assignees
@kat-kan @Slawomir-DKl
Labels
enhancement New feature or request fix fix

Comments

@Slawomir-DKl
Copy link
Collaborator

✏️ Description

Method toISOString() creates a date in the format with milliseconds, and GAD requires a format without milliseconds. Therefore the test is always green, regardless of the operator used in the line date.setFullYear(date.getFullYear() + 1); (you can change + with - and still get the 422 code, even if the date is from the past).
The solution is to add .split('.')[0] + 'Z' after the toISOString() method, e.g.:

const futureDateFormatted: string = date.toISOString().split('.')[0] + 'Z';

Instead we could use the variables customDate.pastDate and customDate.futureDate I've created in Pull Request no. 96 (of course, after merging the latter PR into main branch)

✅ Problem report checklist

  • I created a descriptive title for this problem report
  • I checked that a similar issue hasn't been reported yet
  • I assigned the problem report or added appropriate labels
@Slawomir-DKl Slawomir-DKl added the fix fix label May 5, 2024
@kat-kan
Copy link
Owner

kat-kan commented May 31, 2024

@Slawomir-DKl can you please double check on Your side?
While I agree using the generator for dates is much better approach and I am happy to change test to use it, I can not reproduce the problem with dates on the newest version of GAD.
It appears to me that GAD accepts both formats
image

@Slawomir-DKl
Copy link
Collaborator Author

@kat-kan You're right. In the latest version of GAD (2.6.5) the date validation patterns were fixed - see the Release notes. Therefore we can leave toISOString() method as it will be working from now on (in fact the method alone will be better solution now because it would allow to detect potential problems with date validation during future regression tests)

@kat-kan kat-kan mentioned this issue Jun 1, 2024
Merged
3 tasks
@kat-kan kat-kan linked a pull request Jun 1, 2024 that will close this issue
Fix/issue 97 fix future date test from articles post file #102
Merged
3 tasks
@kat-kan
Copy link
Owner

kat-kan commented Jun 1, 2024

@Slawomir-DKl thank You for checking this, indeed it's mentioned in release notes, I didn't notice
I spent some time trying to include this new feature in our tests. Please let me know what You think!

@kat-kan kat-kan added the enhancement New feature or request label Jun 1, 2024
@Slawomir-DKl
Copy link
Collaborator Author

@kat-kan Nice solution. Thanks to this we can test for future changes in the GAD.
What about moving generating pastDates and futureDates to date.generator.ts file? This way we could use them also in other tests without replicating the whole block of const and push.

@kat-kan
Copy link
Owner

kat-kan commented Jul 7, 2024

@Slawomir-DKl I've just added Your suggestion :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@kat-kan kat-kan

@Slawomir-DKl Slawomir-DKl

Labels
enhancement New feature or request fix fix
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Fix/issue 97 fix future date test from articles post file kat-kan/kawqa-gad-playwright
2 participants
@kat-kan @Slawomir-DKl