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

Improved the way to test your code #1233

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

RealYusufIsmail
Copy link
Contributor

Checklist

Changelog

  • Added a sample class to make it easy to copy and test.
  • Added a folder where the test class can be created and classes apart from SampleTestBot are ignored

Breaking Changes

Description

Closes: NaN

Footnotes

  1. At least started a running bot instance with your changes and triggered an event so your changed code gets executed.

@RealYusufIsmail RealYusufIsmail force-pushed the testing branch 3 times, most recently from aa308e5 to aa27891 Compare April 7, 2023 14:48
@felldo
Copy link
Member

felldo commented Apr 12, 2023

Did you test your recommended way to get started testing own changes?
When I do what you wrote, I get an exception
java.lang.NoClassDefFoundError: Could not initialize class org.codehaus.groovy.vmplugin.v7.Java7

@RealYusufIsmail
Copy link
Contributor Author

Did you test your recommended way to get started testing own changes? When I do your what you wroteI get an exception java.lang.NoClassDefFoundError: Could not initialize class org.codehaus.groovy.vmplugin.v7.Java7

That does not seem an issue with the test

@RealYusufIsmail
Copy link
Contributor Author

I ran the build and it worked

@RealYusufIsmail
Copy link
Contributor Author

Did you test your recommended way to get started testing own changes? When I do your what you wroteI get an exception java.lang.NoClassDefFoundError: Could not initialize class org.codehaus.groovy.vmplugin.v7.Java7

can you tell me what I did

@felldo
Copy link
Member

felldo commented Apr 12, 2023

I ran the build and it worked

I just did what you described. Copied the class in the same directory, right-clicked the file and executed it. Not sure what I would be doing wrong. To verify if it's only my fault, please try to execute a bot in the same way as I did.

@RealYusufIsmail
Copy link
Contributor Author

I ran the build and it worked

I just did what you described. Copied the class in the same directory, right-clicked the file and executed it. Not sure what I would be doing wrong. To verify if it's only my fault, please try to execute a bot in the same way as I did.

I mean I don't have a bot token at hand right now

@RealYusufIsmail
Copy link
Contributor Author

but did you change the class name and set a token

@felldo
Copy link
Member

felldo commented Apr 12, 2023

I also ran the normal tests and it also threw an error. For some reason it seems like gradle doesn't like running with Java 17 in Javacord. When I switched to 11 it worked fine.
And regarding executing the test class, it seems suboptimal that the DEBUG logs are shown in the console, so the logger configuration has to be adjusted or better said seperated so each test directory has their own configuration.

@RealYusufIsmail
Copy link
Contributor Author

Will see if I am able to do the latter part of your message

@RealYusufIsmail
Copy link
Contributor Author

I also ran the normal tests and it also threw an error. For some reason it seems like gradle doesn't like running with Java 17 in Javacord. When I switched to 11 it worked fine.

And regarding executing the test class, it seems suboptimal that the DEBUG logs are shown in the console, so the logger configuration has to be adjusted or better said seperated so each test directory has their own configuration.

Would it be alright to add a .xml file for logging like https://github.com/YDWK/YDWK/blob/master/src/test/resources/logback.xml in the test folder. It says what logging you want and where from.

@felldo
Copy link
Member

felldo commented Apr 15, 2023

Would it be alright to add a .xml file for logging like https://github.com/YDWK/YDWK/blob/master/src/test/resources/logback.xml in the test folder. It says what logging you want and where from.

We already have a logging file and this will most likely not work because you will also affect the logging level for the normal tests, that's why I said the configuration has to be separated somehow.

@RealYusufIsmail
Copy link
Contributor Author

Would it be alright to add a .xml file for logging like https://github.com/YDWK/YDWK/blob/master/src/test/resources/logback.xml in the test folder. It says what logging you want and where from.

We already have a logging file and this will most likely not work because you will also affect the logging level for the normal tests, that's why I said the configuration has to be separated somehow.

Can we leave that for a separate pr

@RealYusufIsmail RealYusufIsmail force-pushed the testing branch 2 times, most recently from 8c07d95 to 5d7c3f1 Compare April 16, 2023 10:36
@felldo
Copy link
Member

felldo commented Apr 16, 2023

Can we leave that for a separate pr

But then what's the point of this PR if it is not really suitable for getting started with testing? Then, I think using the current way to create a test class is easier than having to deal with the logging configuration.

@RealYusufIsmail
Copy link
Contributor Author

Can we leave that for a separate pr

But then what's the point of this PR if it is not really suitable for getting started with testing? Then, I think using the current way to create a test class is easier than having to deal with the logging configuration.

but this serves as a template for a test class and to know where to test. Makes it easer.

@RealYusufIsmail
Copy link
Contributor Author

Can we leave that for a separate pr

But then what's the point of this PR if it is not really suitable for getting started with testing? Then, I think using the current way to create a test class is easier than having to deal with the logging configuration.

but this serves as a template for a test class and to know where to test. Makes it easer.

I could look at how to make the logging work

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