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

pkp/pkp-lib#4246 Add option to select reviewers from last round #3331

Merged
merged 2 commits into from
Mar 17, 2022

Conversation

NateWr
Copy link
Contributor

@NateWr NateWr commented Mar 17, 2022

No description provided.

@CLAassistant
Copy link

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you all sign our Contributor License Agreement before we can accept your contribution.
0 out of 2 committers have signed the CLA.

❌ forgive38
❌ NateWr
You have signed the CLA already but the status is still pending? Let us recheck it.

@Vitaliy-1
Copy link
Contributor

@NateWr, it looks like this submodule update broke tests.

@asmecher
Copy link
Member

^ yep, here's a temporary work-around for pkp-lib:

diff --git a/cypress/support/commands.js b/cypress/support/commands.js
index 02464a43f..137c68f41 100644
--- a/cypress/support/commands.js
+++ b/cypress/support/commands.js
@@ -458,7 +458,7 @@ Cypress.Commands.add('checkEmailTemplateVariables', (selector) => {
  * @param string selector An HTML element selector for a parent element that contains the email composer to check
  */
 Cypress.Commands.add('waitForEmailTemplateToBeLoaded', (selector) => {
-	cy.get(selector);
+	cy.wait(4000); // FIXME: Should be `cy.get(selector);` but the selector is currently missing
 	cy.get(selector + ' .composer__loadingTemplateMask').should('not.exist');
 });

@Vitaliy-1
Copy link
Contributor

Problems I'm experiencing so far:

get div.pkpStatsgraph table tbody tr th:contains("January 29, 2022")
log January 29, 2022
get div.pkpStatsgraph table tbody tr th:contains("January 30, 2022")
log January 30, 2022
get div.pkpStatsgraph table tbody tr th:contains("January 31, 2022")
log January 31, 2022
get div.pkpStatsgraph table tbody tr th:contains("February  1, 2022")

@NateWr
Copy link
Contributor Author

NateWr commented Mar 21, 2022

Are these test failures happening on Travis or just locally? The tests passed on this PR before I merged.

@Vitaliy-1
Copy link
Contributor

Yes, and it's weird. According to the commit history, they are failing after this commit: 27933b0, e.g.: https://app.travis-ci.com/github/pkp/ojs/jobs/563738700 and I can reproduce it locally.

@NateWr
Copy link
Contributor Author

NateWr commented Mar 21, 2022

I see the problem. Working on a fix now: pkp/pkp-lib#7768 (comment)

@asmecher
Copy link
Member

When comparing dates here: https://github.com/pkp/pkp-lib/blob/01303a10c72feabb225c658d16119381e34745fb/cypress/support/commands.js#L614, extra space appears between a month and day if the latter has only one digit; this may be related to the recent date & time formatting switch from strftime:

get div.pkpStatsgraph table tbody tr th:contains("January 29, 2022")
log January 29, 2022
get div.pkpStatsgraph table tbody tr th:contains("January 30, 2022")
log January 30, 2022
get div.pkpStatsgraph table tbody tr th:contains("January 31, 2022")
log January 31, 2022
get div.pkpStatsgraph table tbody tr th:contains("February  1, 2022")

This was actually due to a work-around for a problem that went away with the move from strftime (which space-padded single digit day numbers) to date (which doesn't). I have removed the work-around and the tests work locally; I'll watch the Travis tests to make sure. Commit: pkp/pkp-lib@1a0f3c5

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