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

e2e: Proposal Create mock structure. #2676

Merged

Conversation

vibros68
Copy link
Contributor

@vibros68 vibros68 commented Nov 27, 2021

Closes #2644

This diff create mock data for record/new endpoint. So when creating
proposal e2e test is running. The test will not send the request to the
server. And complete the e2e test for creating normal proposal. Include
select domain and pickup datepickers

@vibros68 vibros68 changed the title [WIP] e2e: Proposal Create mock structure. e2e: Proposal Create mock structure. Nov 28, 2021
@vibros68
Copy link
Contributor Author

@victorgcramos can you review this pr?

Copy link
Member

@victorgcramos victorgcramos left a comment

Choose a reason for hiding this comment

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

records/v1/new endpoint mock looks good, mate. Thanks! Left some inline comments.

Good to remind that mocking the records/new endpoint does not close the full issue, as It expected the whole proposal/create flow to be mocked so we don't need to be connected to the backend.

Comment on lines 127 to 133
export function newRecordSummaryReply({ requestParams: { tokens = [] } }) {
const summaries = tokens.reduce((sum, token) => {
sum[token] = { status: "unvetted" };
return sum;
}, {});
return { summaries };
}
Copy link
Member

Choose a reason for hiding this comment

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

records summaries does not exist. Could you remove it, please?

Comment on lines 2 to 5
import {
PROPOSAL_SUMMARY_STATUS_ACTIVE,
PROPOSAL_SUMMARY_STATUS_UNVETTED
} from "../../utils";
Copy link
Member

Choose a reason for hiding this comment

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

Unused variables

@@ -336,6 +339,8 @@ const ProposalForm = React.memo(function ProposalForm({
)}
<SelectField
name="domain"
id="domain-selector"
dataTestid="domain-selector"
Copy link
Member

Choose a reason for hiding this comment

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

is this used elsewhere?

victorgcramos and others added 5 commits November 29, 2021 19:18
This commit adds the `verifyIdentity` option to user login and me
requests, and improves the proposal create tests to avoid reaching any
backend endpoints.
Improvements for Proposal Create mock
Copy link
Member

@victorgcramos victorgcramos left a comment

Choose a reason for hiding this comment

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

Looking better now. Tests are working as expected, and thanks for adding an extra test case.

I have some small observations regarding the test cases descriptions: they should start with "should". Left one inline suggestion regarding it, which you should replicate for all test cases you create.

Left some other inline suggestions 😁

cy.findByRole("button", { name: /submit/i }).click();
// needs more time in general to complete this request so we increase the
// responseTimeout
cy.wait("@newProposal", { timeout: 10000 }).should((xhr) => {
Copy link
Member

Choose a reason for hiding this comment

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

try using cy.wait("@records.new") instead, and see if you can decrease this timeout. 10s timeout is too much.

@@ -336,6 +337,7 @@ const ProposalForm = React.memo(function ProposalForm({
)}
<SelectField
name="domain"
id="domain-selector"
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
id="domain-selector"
id="proposal-domain-selector"

@@ -149,22 +158,24 @@ Cypress.Commands.add("approveProposal", ({ token }) =>
Cypress.Commands.add("typeCreateProposal", (proposal) => {
cy.server();
cy.findByTestId("proposal-name").type(proposal.name);
cy.findByTestId("text-area").type(proposal.description);
cy.findByTestId("proposal-amount").type(String(proposal.amount / 100)); // get dollars from cents.
Copy link
Member

Choose a reason for hiding this comment

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

Prettier: 80 max char per line.

@@ -101,7 +101,32 @@ export function detailsReply({
return { record };
}

/**
* newRecordReply is the reply to the new command. It returns a new record for the
* request data with given `files`, `publickey`, `signature` and `author` testParams.
Copy link
Member

Choose a reason for hiding this comment

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

The parameters from this replier doc is different from the replier's parameters.

@@ -1,4 +1,4 @@
import { Record, File, Inventory } from "./generate";
import { Record, File, Inventory, Proposal } from "./generate";
Copy link
Member

Choose a reason for hiding this comment

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

Proposal is never used and does not exist on generate scope.

describe("Proposal Create", () => {
// XXX This test needs changes in the Datepicker and (probably) the Select
// components, in order to fill the new form fields such as: start & end dates
// and amount - issue to track <insert issue link>
//
/*it("Paid user can create proposals manually", () => {
it("Paid user can create proposals manually", () => {
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
it("Paid user can create proposals manually", () => {
it("should be able to create proposals", () => {

Copy link
Member

@victorgcramos victorgcramos left a comment

Choose a reason for hiding this comment

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

I'm getting an error on your test run. Please fix it :D

Screen Shot 2021-12-02 at 12 15 34 PM

teste2e/cypress/support/commands.js Outdated Show resolved Hide resolved
teste2e/cypress/support/commands.js Outdated Show resolved Hide resolved
Copy link
Member

@victorgcramos victorgcramos left a comment

Choose a reason for hiding this comment

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

Looking really good. Left just one minor inline comment, and we should be good to go! Nice job @vibros68

publickey: user.publickey,
signature,
publickey: publickey || user.publickey,
recordSignature,
Copy link
Member

Choose a reason for hiding this comment

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

Noticed some issues here. RecordMetadata has no recordSignature field. Actually, it's signature. Also, the signature should have a default value.

@@ -15,7 +15,7 @@ describe("Proposal Create", () => {
// components, in order to fill the new form fields such as: start & end dates
// and amount - issue to track <insert issue link>
//
it("Paid user can create proposals manually", () => {
it("should be able to create proposals", () => {
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
it("should be able to create proposals", () => {
it("should allow paid user to create proposals", () => {

Copy link
Member

@victorgcramos victorgcramos left a comment

Choose a reason for hiding this comment

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

tACK!

Copy link
Member

@tiagoalvesdulce tiagoalvesdulce left a comment

Choose a reason for hiding this comment

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

LGTM

@tiagoalvesdulce tiagoalvesdulce merged commit 19e2970 into decred:master Dec 7, 2021
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.

e2e: Proposal Create mock structure.
4 participants