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

Update javascript image to include java to support selenium #97

Merged
merged 2 commits into from
Oct 7, 2021

Conversation

kharyam
Copy link
Contributor

@kharyam kharyam commented Oct 1, 2021

Purpose

Currently, the javascript container image does not contain the java executable. There are cases where java is required, most notably when running UAT tests via selenium.

# Note: selenium is a common framework used for user interface testing and UAT.
#       The npm selenium web driver package requires java in order to execute
#       selenium tests. For this reason, java is used as the base image

Breaking?

No

Integration Testing

@kharyam kharyam requested review from itewk and dwinchell October 1, 2021 17:47
@kharyam kharyam changed the title WIP: Update javascript image to include java Update javascript image to include java Oct 1, 2021
@itewk
Copy link
Contributor

itewk commented Oct 1, 2021

@itewk itewk added the enhancement New feature or request label Oct 1, 2021
@jkeam
Copy link
Member

jkeam commented Oct 1, 2021

I gotta say I love the emojis in the step names 🎉👍👻

@itewk
Copy link
Contributor

itewk commented Oct 1, 2021

I gotta say I love the emojis in the step names 🎉👍👻

is it a github workflow if it doesn't have emojoies in it?

@itewk itewk changed the title Update javascript image to include java Update javascript image to include java to support selenium Oct 4, 2021
Copy link
Contributor

@itewk itewk left a comment

Choose a reason for hiding this comment

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

@kharyam can you add the note to https://github.com/ploigos/ploigos-containers/blob/main/ploigos-tool-javascript/README.md

@kharyam bumping this. Also can you update the commit message to include "... to support selenium testing" or something to that affect?

@kharyam kharyam force-pushed the feature/npm-java branch 3 times, most recently from 9f89ac1 to bc75473 Compare October 5, 2021 21:04
@kharyam
Copy link
Contributor Author

kharyam commented Oct 5, 2021

@kharyam can you add the note to https://github.com/ploigos/ploigos-containers/blob/main/ploigos-tool-javascript/README.md

@kharyam bumping this. Also can you update the commit message to include "... to support selenium testing" or something to that affect?

Updated, thanks!

@itewk
Copy link
Contributor

itewk commented Oct 6, 2021

@kharyam if you didnt see, looks like some build error for the maven images. but i didn't think you touched those....maybe something broke outside of your PR?

@itewk
Copy link
Contributor

itewk commented Oct 6, 2021

@kharyam seems like we should do the maven version change in a seperate PR, or at least make it a seperate commit. seems to not be "the fault of this change" but just an issue blocking you. sorry for the pain.

@kharyam
Copy link
Contributor Author

kharyam commented Oct 6, 2021

@kharyam if you didnt see, looks like some build error for the maven images. but i didn't think you touched those....maybe something broke outside of your PR?

Thanks @itewk - maven version 3.8.1 is no longer available, bumped to the latest patch release 3.8.3

@kharyam kharyam requested a review from itewk October 6, 2021 15:41
@itewk
Copy link
Contributor

itewk commented Oct 7, 2021

@kharyam if you didnt see, looks like some build error for the maven images. but i didn't think you touched those....maybe something broke outside of your PR?

Thanks @itewk - maven version 3.8.1 is no longer available, bumped to the latest patch release 3.8.3

Heyo @kharyam sory to be a pain, but can you seperate the maven change into its own commit, can leave it in this PR for simplicity if you want, but at least having a clear history of when we bumped maven version would be good.

@kharyam
Copy link
Contributor Author

kharyam commented Oct 7, 2021

@kharyam if you didnt see, looks like some build error for the maven images. but i didn't think you touched those....maybe something broke outside of your PR?

Thanks @itewk - maven version 3.8.1 is no longer available, bumped to the latest patch release 3.8.3

Heyo @kharyam sory to be a pain, but can you seperate the maven change into its own commit, can leave it in this PR for simplicity if you want, but at least having a clear history of when we bumped maven version would be good.

@itewk no problem - done

@itewk itewk merged commit f2a18eb into main Oct 7, 2021
@itewk itewk deleted the feature/npm-java branch October 7, 2021 12:57
@itewk
Copy link
Contributor

itewk commented Oct 7, 2021

@kharyam thanks

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants