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

Added JWT-PreProcessor plugin details and wiki page. #608

Merged
merged 5 commits into from
Aug 15, 2023

Conversation

sashikaR
Copy link
Contributor

Hi Team,

I developed a Jmeter PreProcessor plugin for JWT generation and I would like to publish it under the Jmeter-plugin manager. Please review my pull request.
Plugin Repo : https://github.com/sashikaR/jwt-preProcessor

Copy link
Owner

@undera undera left a comment

Choose a reason for hiding this comment

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

Please address my comments

@@ -0,0 +1,12 @@
= JAVA WEB TOKEN(JWT) Pre Processor =
Copy link
Owner

Choose a reason for hiding this comment

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

Please remove this file. The documentation is served from your repo (pointed via helpUrl)

},
{
"id": "jwt-preProcessor",
"name": "JWT-PreProcessor",
Copy link
Owner

Choose a reason for hiding this comment

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

Please use space in name instead of -, it will look better for users.

"markerClass": "sr.jmeter.jwt.preprocessor.JwtPreProcessor",
"installerClass": "sr.jmeter.jwt.preprocessor.JwtPreProcessor",
"versions": {
"v1.0.0": {
Copy link
Owner

Choose a reason for hiding this comment

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

please don't use v prefix in mapping key

Copy link
Contributor Author

@sashikaR sashikaR left a comment

Choose a reason for hiding this comment

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

Updated as per the review comments. Please review the PR.

@undera
Copy link
Owner

undera commented Aug 14, 2023

Look at the CI failure. Can you compile your plugin using older JDK requirement? This will ensure more people can use it.

@sashikaR
Copy link
Contributor Author

Look at the CI failure. Can you compile your plugin using older JDK requirement? This will ensure more people can use it.

Can I know the Java version which I should compile and test?

@undera
Copy link
Owner

undera commented Aug 14, 2023

The recommended version is Java 11

@sashikaR
Copy link
Contributor Author

Hi, Please review the PR with the new build.

Added a new for plugin-manager
@sashikaR
Copy link
Contributor Author

Hi, Updated the Manifest file in the plugin build. Please re-run the build.

@sashikaR
Copy link
Contributor Author

Hi undera,

I need some guidance from you to resolve this issue. The class which I gave in markerClass is the implementation of PreProcessor interface and AbstractTestElement class in my plugin. So there is no need to define the main method in that class since it is trigger by the Jmeter application. Could you please guide me what is the class I should give in markerClass and installerClass?

Previously in my plugin's manifest included the Main-Class and I changed the build to remove it now. So not sure why the build is failing with main method is not found error.

Thank you.

@undera
Copy link
Owner

undera commented Aug 15, 2023

If your plugin does not have custom installation procedures, you can just omit installerClass from descriptor

@sashikaR
Copy link
Contributor Author

Hi,
Please run the build now. I removed the installerClass as suggested.

Thanks.

@codecov
Copy link

codecov bot commented Aug 15, 2023

Codecov Report

Merging #608 (24e5fa1) into master (f6eea45) will not change coverage.
The diff coverage is n/a.

@@            Coverage Diff            @@
##             master     #608   +/-   ##
=========================================
  Coverage     68.87%   68.87%           
  Complexity     2598     2598           
=========================================
  Files           230      230           
  Lines         15721    15721           
  Branches       1612     1612           
=========================================
  Hits          10828    10828           
  Misses         4093     4093           
  Partials        800      800           

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@undera
Copy link
Owner

undera commented Aug 15, 2023

Looks good now.
Thanks for publishing your plugin!

@undera undera merged commit e0c6fe2 into undera:master Aug 15, 2023
3 checks passed
@sashikaR
Copy link
Contributor Author

sashikaR commented Aug 15, 2023 via email

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