Skip to content

Conversation

@htdat
Copy link
Member

@htdat htdat commented Jun 9, 2021

Fix #652

Description

The root cause is described here #652 (comment)

To fix this issue, I am changing the way selecting a future date for schedulePost:

  • Calculate the date in 2 weeks since today.
  • Use DOM selector and update the respective values for day, month, year.

Some notes:

Steps to Test

If possible, we can use the diff in this comment #652 (comment) to generate new screenshots with this fix.

With this fix, the date of the post is expected now:

@htdat htdat requested review from mikeyarce, natebot and nielslange June 9, 2021 10:46
@htdat htdat self-assigned this Jun 9, 2021
htdat added 2 commits June 10, 2021 11:02
It's similar to the current way this file is doing
@htdat
Copy link
Member Author

htdat commented Jun 10, 2021

In case, it helps, here is my screen record for the specific failed test with this fix:

https://d.pr/v/kWnVMx

nielslange
nielslange previously approved these changes Jun 10, 2021
Copy link
Contributor

@nielslange nielslange left a comment

Choose a reason for hiding this comment

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

LGTM!

I tested this PR using the following steps:

  1. Start Docker
  2. Run npm run wp-env start
  3. Run npm run test-e2e
  4. One test case is failing
> edit-flow@2.0.0 test-e2e
> wp-scripts test-e2e --config tests/e2e/jest.config.js

 FAIL  tests/e2e/specs/calendar-body.test.js (25.717s)
  ● Calendar Body › expects a scheduled post can be dragged and dropped

    expect(received).not.toContain(expected) // indexOf

    Expected value: not "Scheduled Post"
    Received array:     ["Scheduled Post", "Published Post"]

      94 |         expect(await scheduledPostDay.evaluate((node) => {
      95 |             return Array.prototype.slice.call(node.querySelectorAll('.item-headline.post-title strong')).map((n) => n.innerText)
    > 96 |         })).not.toContain('Scheduled Post');
         |                 ^
      97 |     });
      98 | });

      at _callee3$ (specs/calendar-body.test.js:96:17)
      at asyncGeneratorStep (specs/calendar-body.test.js:7:103)
      at _next (specs/calendar-body.test.js:9:194)
          at runMicrotasks (<anonymous>)
      at processTicksAndRejections (../../node:internal/process/task_queues:96:5)

 PASS  tests/e2e/specs/editorial-comments.test.js (7.023s)
 PASS  tests/e2e/specs/calendar-header.test.js (5.558s)
 PASS  tests/e2e/specs/simple.test.js

Test Suites: 1 failed, 3 passed, 4 total
Tests:       1 failed, 5 passed, 6 total
Snapshots:   0 total
Time:        39.61s, estimated 40s
Ran all test suites.
  1. Check out this PR
  2. Run npm run test-e2e
  3. All test cases are successful
> edit-flow@2.0.0 test-e2e
> wp-scripts test-e2e --config tests/e2e/jest.config.js

 PASS  tests/e2e/specs/calendar-body.test.js (25.829s)
 PASS  tests/e2e/specs/calendar-header.test.js (5.989s)
 PASS  tests/e2e/specs/simple.test.js
 PASS  tests/e2e/specs/editorial-comments.test.js (6.472s)

Test Suites: 4 passed, 4 total
Tests:       6 passed, 6 total
Snapshots:   0 total
Time:        39.641s
Ran all test suites.

Copy link
Contributor

@mikeyarce mikeyarce left a comment

Choose a reason for hiding this comment

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

Looks good to me! I just had a small nitpick about one of the comments.

Thanks @htdat - so nice to see that green checkmark next to all the tests!

@htdat htdat merged commit 12b2f8e into master Jun 14, 2021
@htdat htdat deleted the fix/652 branch June 14, 2021 03:57
@htdat
Copy link
Member Author

htdat commented Jun 14, 2021

Thanks @nielslange and @mikeyarce! I've merged this PR after updating the comment :D

@htdat htdat changed the title Update the way selecting a future date for schedulePost E2E: Update the way selecting a future date for schedulePost Jul 1, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

E2E test - calendar-body.test.js

4 participants