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

Implement registerUI #18

Merged
merged 54 commits into from
Aug 3, 2021
Merged

Implement registerUI #18

merged 54 commits into from
Aug 3, 2021

Conversation

rob-3
Copy link
Contributor

@rob-3 rob-3 commented Aug 1, 2021

This PR implements the new registerUI function and makes a few breaking changes. Future Commands must destructure off their arguments as in

async run({ interaction, registerUI }) {

rather than simply taking in the interaction parameter.

This doesn't break the standard method of event handling with Collectors.

There are also numerous improvements to the core including extracting the interaction listening from the Client class, which is better suited for managing the updating and synchronization of commands with the network.

@suneettipirneni
Copy link
Member

Just for assurance, has this been tested thoroughly?

@rob-3
Copy link
Contributor Author

rob-3 commented Aug 2, 2021

Probably not as extensively as I would like. I tested it with a rewritten vibe and roles command and resolved the issues I found, but mostly left other commands untouched.

@rob-3
Copy link
Contributor Author

rob-3 commented Aug 2, 2021

I keep thinking about how we can test our code in an automated way without mocking nearly everything out and I'm drawing a blank.

@suneettipirneni
Copy link
Member

I keep thinking about how we can test our code in an automated way without mocking nearly everything out and I'm drawing a blank.

You can't you're relying on an external api for state. You can take a look at my testing branch, to see what I've done in terms of mocking.

@rob-3
Copy link
Contributor Author

rob-3 commented Aug 2, 2021

I guess one idea is to have a separate testing bot that uses discord.js to invoke commands for itself and then checks for results, but that seems like a substantial amount of effort in itself. And I'm not sure there's any practical way to automatically click a button or select options in a menu.

@rob-3
Copy link
Contributor Author

rob-3 commented Aug 2, 2021

Ok yeah the testing branch looks like what probably is the best testing solution in terms of achievability. I just wonder if all the work mocking things out is really worthwhile for most commands. Tbh we haven't run into a ton of bugs overall. Also I always feel like the more things that are mocked, the more room there is in production for bugs to creep into the real implementations.

@rob-3
Copy link
Contributor Author

rob-3 commented Aug 2, 2021

That being said, I can write in some unit tests for a lot of the mostly pure functions like registerUI() that don't really need to talk to the outside world.

@rob-3 rob-3 merged commit 0595e09 into main Aug 3, 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.

2 participants