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

Hoist up Node Gradle plugin, set TestTomcat baseDir, add hasContentType matcher, HTTP status consts #3

Closed
wants to merge 3 commits into from

Conversation

mbland
Copy link
Owner

@mbland mbland commented Nov 25, 2023

Combining several small touch up changes in one:

Hoist Node Gradle plugin up to settings.gradle.kts

settings.gradle.kts seems a better place to specify the versions of Gradle plugins than in the project specific build.gradle.kts files.

Set TestTomcat baseDir explicitly

This is in preparation for possibly launching a TestTomcat per test method, instead of per test fixture/suite.

I found out that trying to dynamically create a baseDir for each instance led to the first instance's baseDir reappearing. I haven't been able to see clearly how this is happening, but it seems that:

  1. The baseDir isn't strictly necessary in our case, since we're not running JSPs.
  2. Using the same build/test-tomcat baseDir for all instances and having each instance delete it when finished isn't breaking anything.

Add hasContentType matcher, HTTP status consts

This matcher removes a lot of boilerplate from checking the Content-Type of a HttpResponse. It's based on the Hamcrest API with the help of these tutorials:

This could be generalized to handle any header. However, I want to keep it straightforward for now, as a good basic example of how to write a custom matcher.

Also took the opportunity to replace bare HTTP status codes (200, et. al.) with constants defined on jakarta.servlet.http.HttpServletResponse.

@mbland
Copy link
Owner Author

mbland commented Nov 25, 2023

Closing and reopening to try to trigger checks...

@mbland mbland closed this Nov 25, 2023
@mbland
Copy link
Owner Author

mbland commented Nov 25, 2023

Reopening to trigger checks...

@mbland
Copy link
Owner Author

mbland commented Nov 25, 2023

Switched the branch protection rule to use actions from "GitHub Actions" instead of "any source" to see if that'll trigger the build...

@mbland mbland closed this Nov 25, 2023
@mbland
Copy link
Owner Author

mbland commented Nov 25, 2023

And reopening again...

@mbland mbland reopened this Nov 25, 2023
mbland added a commit that referenced this pull request Nov 25, 2023
I could not get PR #3 to run the Run Tests action after updating the
branch protection rule, force-pushing, or closing/reopening the PR. I
think it's because the updates in PR #2 were too restrictive.

Once I commit this, I'll try to trigger PR #3 again.
settings.gradle.kts seems a better place to specify the versions of
Gradle plugins than in the project specific build.gradle.kts files.
This is in preparation for possibly launching a TestTomcat per test
method, instead of per test fixture/suite.

I found out that trying to dynamically create a baseDir for each
instance led to the first instance's baseDir reappearing. I haven't been
able to see clearly how this is happening, but it seems that:

1. The baseDir isn't strictly necessary in our case, since we're not
   running JSPs.
2. Using the same build/test-tomcat baseDir for all instances and having
   each instance delete it when finished isn't breaking anything.
This matcher removes a lot of boilerplate from checking the Content-Type
of a HttpResponse. It's based on the Hamcrest API with the help of these
tutorials:

- https://hamcrest.org/JavaHamcrest/tutorial
- https://www.baeldung.com/hamcrest-custom-matchers

This could be generalized to handle any header.  However, I want to keep
it straightforward for now, as a good basic example of how to write a
custom matcher.

Also took the opportunity to replace bare HTTP status codes (200, et.
al.) with constants defined on jakarta.servlet.http.HttpServletResponse.
mbland added a commit that referenced this pull request Nov 25, 2023
Effectively reverts commit d694888.

Commit 974b1e0 didn't help PR #3 run.
@mbland
Copy link
Owner Author

mbland commented Nov 25, 2023

I'll try closing and reopening...

@mbland mbland closed this Nov 25, 2023
@mbland
Copy link
Owner Author

mbland commented Nov 25, 2023

...one more time...

@mbland mbland reopened this Nov 25, 2023
@mbland
Copy link
Owner Author

mbland commented Nov 25, 2023

OK, I'll close this one and reopen a new one to see if that helps.

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.

1 participant