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

[WALL] Lubega / WEBREL-2549 / fix failing tests on staging and master #13892

Conversation

lubega-deriv
Copy link
Contributor

Changes:

  • Removed moment dependency
  • Updated day to 15 instead of current day to avoid issue of having double elements with the same day at the end of the month

Copy link

vercel bot commented Feb 29, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Updated (UTC)
deriv-app ✅ Ready (Inspect) Visit Preview Feb 29, 2024 5:21am

Copy link

sonarcloud bot commented Feb 29, 2024

Quality Gate Passed Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
No data about Coverage
No data about Duplication

See analysis details on SonarCloud

Copy link
Contributor

@adrienne-deriv adrienne-deriv left a comment

Choose a reason for hiding this comment

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

LGTM

@jim-deriv jim-deriv merged commit 5150a8f into deriv-com:rc_V20240226_0 Feb 29, 2024
4 checks passed
Copy link
Contributor

A production App ID was automatically generated for this PR. (log)

Click here to copy & paste above information.
- **PR**: [https://github.com/binary-com/deriv-app/pull/13892](https://github.com/binary-com/deriv-app/pull/13892)
- **URLs**:
    - **w/ App ID + Server**: https://deriv-app-git-fork-lubega-deriv-lubega-webrel-2549fix-fa-c6019b.binary.sx?qa_server=red.derivws.com&app_id=23836
    - **Original**: https://deriv-app-git-fork-lubega-deriv-lubega-webrel-2549fix-fa-c6019b.binary.sx
- **App ID**: `23836`

Copy link
Contributor

🚨 Lighthouse report for the changes in this PR:

Category Score
🔺 Performance 31
🟧 Accessibility 89
🟢 Best practices 92
🟧 SEO 85
🟧 PWA 78

Lighthouse ran with https://deriv-app-git-fork-lubega-deriv-lubega-webrel-2549fix-fa-c6019b.binary.sx/

@@ -98,10 +97,13 @@ describe('DatePicker Component', () => {
const calendarButton = screen.getByTestId('wallets_datepicker_button');
fireEvent.click(calendarButton);

const testDay = moment().format('D');
const dateElements = screen.getAllByText(testDay);
const date = new Date();
Copy link
Contributor

Choose a reason for hiding this comment

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

suggestion for future: when its coming to date, set it to constant.

e.g. const CURRENT_DATE = 24534534534; // constant timestamp
and then:
const data = new Date(CURRENT_DATE);

alternatively, mock whole moment / whoel Date() object with sinon or jest mocking techniques (depending on particular needs)

Copy link
Contributor

Choose a reason for hiding this comment

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

as using "current" date will be prone to errors, due to the fact, that it will be changing.
Especially during important periods like new year, summertimechange, or when running in different parts of the world, or different locales (e.g. will it behave the same way if I run this test locally on laptop in Malta or Dubai or Japan? will it be the same if my default system formatting setting are gonna be different than here? etc etc dozens of cases like that)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@wojciech-deriv not sure if this would work in this scenario, as we're using an external library (react-calendar). By default when opening the calendar it would open the current month. So what is trying to be tested here is that:

  1. Clicking the calendarButton will open up the DatePicker
  2. Click on any day button (in this case '15' to make sure there're no buttons with the same number in view)
  3. OnDateChange is called with the date in format YYYY-MM-DD

By setting a constant date, we would have to mock the behaviour of redirecting to that date (i.e through clicking on the arrow buttons or choose month/year from above) but depending on when the test is being carried out the redirecting steps would change

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.

5 participants