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

Add Basic Integration Tests #11

Merged
merged 6 commits into from
Sep 10, 2024
Merged

Conversation

danclaytondev
Copy link
Contributor

@danclaytondev danclaytondev commented Sep 4, 2024

This PR adds some initial integration tests for the helper function part of this library; by integration tests I mean that I am only tested exported functionality of the library, by creating a Vite Fragment and then checking the contents of that generated HTML fragment.

The tests are based on the instructions in the Vite docs for backend integration, so I have added the manifest they use in those docs, and then I check that each of the HTML tags the docs say should be generated are contained in the fragment we produce with this library.

After writing the tests, they failed initially, as I think we are generating the modulepreload links incorrectly. So I have added a change to fix how we generate the <link> elements for preloading.

@danclaytondev
Copy link
Contributor Author

When writing tests I found that our script tag functionality in dev mode has problems if you add a trailing slash to the ViteURL or use a root/ in your entrpoint. I have implemented the function from net/url which joins the base URL to a path with any combination of slashes, and always (hopefully) produces a valid URL.

Copy link
Owner

@olivere olivere left a comment

Choose a reason for hiding this comment

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

I just see two minor improvements.

.github/workflows/go.yml Outdated Show resolved Hide resolved
.github/workflows/go.yml Show resolved Hide resolved
manifest.go Show resolved Hide resolved
Co-authored-by: Oliver Eilhard <oliver@eilhard.net>
Copy link
Owner

@olivere olivere left a comment

Choose a reason for hiding this comment

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

LGTM. Thank you. 🎈

Will merge in a minute.

@olivere olivere merged commit a7a77c1 into olivere:main Sep 10, 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.

2 participants