Skip to content

Conversation

@ericsciple
Copy link
Contributor

No description provided.

uses: actions/checkout@v2

- name: Setup Node.js 12.x
- name: Setup Node.js 12
Copy link
Contributor Author

Choose a reason for hiding this comment

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

i removed the empty lines between steps since the file is much larger now (tighten up individual jobs, to make file more skimmable)

http_proxy: http://localhost:3128
https_proxy: http://localhost:3128

test:
Copy link
Contributor Author

Choose a reason for hiding this comment

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

added E2E test job that leverages ./


- name: Lint
run: npm run format-check
test-proxy:
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Switched the proxy test to:

  1. Run in a container, with dns disabled (options: --dns 127.0.0.). This guarantees that all requests are actually going through the proxy. Previous approach hacked hosts file which does not guarantee all traffic.
  2. Switched to widely used squid container image, rather than maintain our own
  3. Switched to testing ./ during proxy test rather than unit tests


- name: npm test
run: npm test
test-bypass-proxy:
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added an E2E proxy bypass test also

@@ -0,0 +1,11 @@
if (!$args.Count -or !$args[0])
Copy link
Contributor Author

Choose a reason for hiding this comment

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

helper script to validate a specific version of java is in the PATH

@ericsciple ericsciple changed the title [draft - not ready] standardize workflow test pattern standardize workflow test pattern Feb 12, 2020
@ericsciple ericsciple force-pushed the users/ericsciple/m166proxy branch from 209a14f to 60cf21b Compare February 12, 2020 00:38
Copy link
Contributor

@konradpabjan konradpabjan left a comment

Choose a reason for hiding this comment

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

LGTM 👍

Really good additions that will help with testing & validation. Testing only one version of Java (the latest, version 13) seems sufficient. No point in kicking off a huge Matrix that is effectively doing the same thing considering there are a lot of versions of Java that we support

@konradpabjan konradpabjan merged commit 5ef3a8d into actions:master Feb 12, 2020
@ericsciple ericsciple deleted the users/ericsciple/m166proxy branch February 12, 2020 14:50
@giltene
Copy link
Contributor

giltene commented Feb 12, 2020

LGTM 👍

Really good additions that will help with testing & validation.

+1

Testing only one version of Java (the latest, version 13) seems sufficient. No point in kicking off a huge Matrix that is effectively doing the same thing considering there are a lot of versions of Java that we support

FYI, I've been keeping an eye on things externally with a pretty full matrix here: https://github.com/giltene/jdkTestingMatrix to make sure the various versions and spellings (e.g. ea, non-ea). Not sure if we need this to be part of the project or if external monitoring is sufficient, but it does serve as an interesting status if/when people observe problems and want to check to see if it is a systemic issue.

tdfacer pushed a commit to ifit/setup-java that referenced this pull request Oct 7, 2025
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.

3 participants