Mainnet liberated integration test fix#4542
Mainnet liberated integration test fix#4542mnaamani merged 19 commits intoJoystream:mainnet-liberatedfrom
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎ 1 Ignored Deployment
|
1489e1c to
207e42b
Compare
mnaamani
left a comment
There was a problem hiding this comment.
Overall looks great, well done. I left my concern about assumption being made about result of proposals query from chain state and query node which will not always hold. So it would be better to address now, otherwise it might be hard to debug failing proposals test in future.
Although running the integration tests locally didn't cause a problem.
| ) | ||
| const executionBlocks = await Promise.all( | ||
| this.proposals.map(async (proposal, i) => { | ||
| const qProposal = qProposals[i] |
There was a problem hiding this comment.
There is a potential problem with this assignment since qProposals will contain all proposals every created, and this.proposals is only the proposals that are currently active in chain state. It would be more correct to pick from qProposals the proposal with the matching proposalId.
|
|
||
| async runQueryNodeChecks(): Promise<void> { | ||
| await super.runQueryNodeChecks() | ||
| // Query the events |
There was a problem hiding this comment.
best to remove commented out code
| // Before any other jobs hire then terminate all WG leads | ||
| const terminateLeadsJob = job('terminate working-group leads', terminateLeads).after( | ||
| job('sudo lead opening', leadOpening(process.env.IGNORE_HIRED_LEADS === 'true')).after(proposalsJob) | ||
| const sudoHireLead = job('sudo lead opening', leadOpening(process.env.IGNORE_HIRED_LEADS === 'true')).after( |
There was a problem hiding this comment.
the "sudo" in the name of this job doesn't make sense anymore
|
Additional points: Also update some of the bash scripts in tests/network-tests/ that still have some reference to sudo arguments and env variables which are not required anymore, and dropping the SUDO_ACCOUNT_URI from .env file |
|
I have addressed all the required comments. |
| await new FixtureRunner(buyMembershipHappyCaseFixture).run() | ||
| const [inviterMemberId] = buyMembershipHappyCaseFixture.getCreatedMembers() | ||
|
|
||
| // Membership WG balance required |
There was a problem hiding this comment.
Since this flow could be used in other scenarios where perhaps a council was not elected, maybe it should fail early.
| const decideOnUpdateWgBudgetProposalStatusFixture = new DecideOnProposalStatusFixture(api, query, [ | ||
| { proposalId: updateWgBudgetProposalId, status: 'Approved', expectExecutionFailure: false }, | ||
| ]) | ||
| await new FixtureRunner(decideOnUpdateWgBudgetProposalStatusFixture).run() |
There was a problem hiding this comment.
Will the fixture fail if there is no council to actually vote on the proposal?
Intro & Scope
Fixes integration tests after the removal of the sudo pallet as prescribed in #4401
Details
mainnet-liberatedTech debt
I think a refactoring is needed in order to encapsulate Proposal Creation + Proposal Execution in one subclass of
BaseQueryNodeFixture. If you think it's urgent I'll do it as part of this PR review iteration, otherwise I'll create a issue for the futureNext steps
Merging the resulting PR into
Ephesus: #4556┆Issue is synchronized with this Asana task by Unito