-
Notifications
You must be signed in to change notification settings - Fork 44
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
Add full end to end integration tests for web3 #10016
base: master
Are you sure you want to change the base?
Conversation
f76429b
to
90b9c17
Compare
# Conflicts: # libs/model/src/chainEventSignatures.ts # packages/commonwealth/server/workers/commonwealthConsumer/policies/chainEventCreated/chainEventCreatedPolicy.ts
# Conflicts: # libs/model/src/database.ts
export async function setupCommonwealthConsumer(): Promise<void> { | ||
export async function setupCommonwealthConsumer( | ||
skipRmqAdapter?: boolean, | ||
): Promise<void> { | ||
let brokerInstance: Broker; | ||
try { | ||
const rmqAdapter = new RabbitMQAdapter( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
do you need to create this when skip is true?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am not sure how the broker works, but from my point of view it just performs checks to ensure you are not instantiating the adapter twice.
We need to instantiate the adapter twice here because we are running these separate services in the same process (So I can get debugging and we won't need to rebuild the docker image each change)
@@ -27,10 +28,9 @@ export const processChainEventCreated: EventHandler< | |||
EvmEventSignatures.Launchpad.TokenLaunched | |||
) { | |||
await command(Token.CreateToken(), { | |||
actor: middleware.systemActor({}), | |||
actor: [] as unknown as Actor, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
not sure what you are trying to do here, systemActor should work
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think you are right. I was thinking we didn't need an actor in general, but we definitely do since the command takes in user defined input like description and image_url.
|
||
export async function setupCommonwealthE2E() { | ||
// reset db | ||
await tester.bootstrap_testing(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
you won't need this after merging xp projector PR
import { GenericContainer, Wait } from 'testcontainers'; | ||
import { config } from '../../../../../server/config'; | ||
|
||
export async function setupRabbitMq() { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks like another RMQ adapter for test containers, you can continue using ports if you use this adapter
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is not the adapter but the init function to start up the rmq in a separate docker container via the testcontainers library.
The adapters connect to this rmq instance.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You can make it an adapter and then call broker(new TestContainerRMQAdapter) the first time
9ed98fe
to
16641bd
Compare
// TODO: Figure out why numUnrelayedEvents is negative. Skipping sleep step causing event queue to clog | ||
if (numUnrelayedEvents <= 0) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No need to change this.
A negative value likely means that the trigger is implemented wrong. If you copied and pasted from the migration, then check for a later migration that fixed the PG trigger.
Link to Issue
Closes: #10001
Description of Changes