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

Fix suggestion for #960 #1061

Closed
wants to merge 4 commits into from
Closed

Fix suggestion for #960 #1061

wants to merge 4 commits into from

Conversation

kumlali
Copy link

@kumlali kumlali commented Jan 29, 2024

Hello,

I simply added " - triggered" or " - resolved" to the end of e-mail's subject regarding the status. Similar feature is already implemented for PagerDuty and Slack alerts.

Although it is not configurable, it would address the requirements listed #960.

Note: I work in an enterprise environment and it is nearly impossible to retrive artifacts and compile the project. (Enterprise Maven repo setting, proxy setting, proxy certificate problems, SSL handshake problems, node-14.21.X-win-x64.zip package downloading problems, missing node.exe execution grants in Maven target directory, conflict with the node installation on the machine etc.) Because, I was only able to compile Java modules -by skipping tests- and failed to compile ui part, there could be other dependencies which I could not aware of.

Regards,

Ali Sadik Kumlali

@kumlali
Copy link
Author

kumlali commented Jan 29, 2024

Seems like some checks are failing due to the scripts that are not changed by this PR:

Error: Failed to execute goal org.apache.maven.plugins:maven-surefire-plugin:3.0.0:test (default-test) on project glowroot-ui: There are test failures.
Error:
Error: Please refer to /home/runner/work/glowroot/glowroot/ui/target/surefire-reports for the individual test results.
Error: Please refer to dump files (if any exist) [date].dump, [date]-jvmRun[N].dump and [date].dumpstream.
Error: -> [Help 1]
Error:
Error: To see the full stack trace of the errors, re-run Maven with the -e switch.
Error: Re-run Maven using the -X switch to enable full debug logging.
Error:
Error: For more information about the errors and possible solutions, please read the following articles:
Error: [Help 1] http://cwiki.apache.org/confluence/display/MAVEN/MojoFailureException
Error:
Error: After correcting the problems, you can resume the build with the command
Error: mvn -rf :glowroot-ui
/usr/bin/bash /home/runner/work/_actions/coactions/setup-xvfb/v1/dist/cleanup.sh
/usr/bin/bash: /home/runner/work/_actions/coactions/setup-xvfb/v1/dist/cleanup.sh: No such file or directory
Error: The process '/usr/bin/xvfb-run' failed with exit code 1

@Nowheresly
Copy link
Collaborator

Hi,
thanks for your PR.
It seems some tests have to be adapted.

expected: "[Glowroot] Test email"
 but was: "[Glowroot] Test email - resolved"
	at java.base/jdk.internal.reflect.NativeConstructorAccessorImpl.newInstance0(Native Method)
	at java.base/jdk.internal.reflect.NativeConstructorAccessorImpl.newInstance(NativeConstructorAccessorImpl.java:62)
	at java.base/jdk.internal.reflect.DelegatingConstructorAccessorImpl.newInstance(DelegatingConstructorAccessorImpl.java:45)
	at org.glowroot.ui.AdminJsonServiceTest.testWithoutDefaults(AdminJsonServiceTest.java:76)


@kumlali
Copy link
Author

kumlali commented Jan 31, 2024

Hi @Nowheresly,

Thanks for directly pointing out to the root cause.

I updated the test cases and all the checks have passed successfully.

@kumlali
Copy link
Author

kumlali commented Feb 2, 2024

Hi @Nowheresly,

Please let me know whether the PR is eligible to merge.

@Nowheresly
Copy link
Collaborator

Yes, it seems good to me.

@Nowheresly
Copy link
Collaborator

Thanks for your contribution.

@Nowheresly Nowheresly closed this Feb 3, 2024
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.

None yet

2 participants