Skip to content

Change resource used for testing resource-response#548

Open
The-Alchemist wants to merge 1 commit intoring-clojure:masterfrom
The-Alchemist:patch-1
Open

Change resource used for testing resource-response#548
The-Alchemist wants to merge 1 commit intoring-clojure:masterfrom
The-Alchemist:patch-1

Conversation

@The-Alchemist
Copy link
Copy Markdown

@The-Alchemist The-Alchemist commented Apr 2, 2026

Change resource used for testing resource-response

When testing with alternative Clojure implementations the Clojure core
namespaces are not guaranteed to be in a jarfile, so change the
resource to a pom.properties file, which we know must be in a jar and
not on the filesystem (for example, via a Leiningen checkout).

@weavejester
Copy link
Copy Markdown
Member

The test you've modified is explicitly checking resources from a jar file are returned as an InputStream, so changing it to check for a File as well defeats the point of the test.

What circumstances are you running the tests under that the clojure.java.io namespace isn't in a jar?

@The-Alchemist
Copy link
Copy Markdown
Author

The-Alchemist commented Apr 2, 2026

Indeed, great point. This is an quite unique oddball case, as I'm working another Clojure fork like Babashka called Cloffle https://github.com/The-Alchemist/cloffle-clojure), so the files are exploded.

I'm using ring as one of the projects to make sure we compare against, and I'm trying to avoid carrying a patch for just this test case (everything else passes).

Maybe there's a better way to achieve this goal?

@weavejester
Copy link
Copy Markdown
Member

The test needs a jar file on the classpath. As I'm unfamiliar with your setup, I'm unsure how best to go about solving this. Are all dependencies exploded, or just Clojure core? Could we just use a different resource?

@The-Alchemist
Copy link
Copy Markdown
Author

The test needs a jar file on the classpath. As I'm unfamiliar with your setup, I'm unsure how best to go about solving this. Are all dependencies exploded, or just Clojure core? Could we just use a different resource?

Just Clojure code is exploded. So we can just check any resource on the classpath. I force-pushed a new change that checks the file META-INF/maven/org.ring-clojure/ring-core-protocols/pom.properties instead. What do you think @weavejester ?

@weavejester
Copy link
Copy Markdown
Member

That seems fine. Can you ensure all lines are under 80 characters and change the commit message to:

Change resource used for testing resource-response

When testing with alternative Clojure implementations the Clojure core
namespaces are not guaranteed to be in a jarfile, so change the
resource to a pom.properties file, which we know must be in a jar and
not on the filesystem (for example, via a Leiningen checkout).

When testing with alternative Clojure implementations the Clojure core
namespaces are not guaranteed to be in a jarfile, so change the
resource to a pom.properties file, which we know must be in a jar and
not on the filesystem (for example, via a Leiningen checkout).

Signed-off-by: The-Alchemist <kap4020@gmail.com>
@The-Alchemist The-Alchemist changed the title test tweak: allow resource-response to return InputStream or File Change resource used for testing resource-response Apr 20, 2026
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