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

chore: rework build and integration tests #30

Merged
merged 8 commits into from
Nov 19, 2024
Merged

Conversation

dav1do
Copy link
Collaborator

@dav1do dav1do commented Nov 15, 2024

I made a lot of changes as I was adding wasm for the flight-sql-client in #29 so I'm pulling out some of the unrelated changes. I reworked the build system quite a bit... open to reverting/changing things but I found some of the install/prepare hook build stuff a bit odd (and the cause of a lot of my grief when build wasn't building my code).

I plan to replace both of these workflows in #29, but splitting out some of these changes that are groundwork for the new package will hopefully make it easier to review.

  • Modified install to only install and not build. Modified build to completely build and not just build types.
  • Reworked the integration tests.
    • Removed the withContainers package and made a local version to make it easier to specify multiple ports for ceramic one (flight sql and API) and generally have more control. I intended to run one image for everything since they can get rate limited, but haven't found a nice way to make that work. So we could probably get a way with 1 port specified per test file if we don't want our own, but it was a tiny package (1 file).
    • To get around the rate limits, that project runs tests serially.
    • Removed the jest global environment stuff and just use beforeAll/afterAll in tests (probably fine to bring back as the build not building is likely why I thought it was all broken), but it seems like overkill for pretty simple set up to me and removes a package
  • Fixed a few small lints and a bug related to default date timezone handling (it worked if you were located in a +0 timezone 😅)

this might be a mistake for CI but the defaults have changed
…er and remove jest env overrides

seems simple to just use beforeAll/afterAll to set up the client since we know what to expect. the withContainer package didn't allow multiple ports, so modified to do so so we can eventually use 1 image for multiple files if we want or use this for other purposes. in the end, didn't really need it since we only need api or flight, technically as it is, but it isn't super complex and lets us modify it as desired
…ly build

this seems more predictable for CI/dev
@dav1do dav1do requested review from smrz2001 and mzkrasner November 15, 2024 19:12
@dav1do dav1do force-pushed the chore/rework-build branch from 002e370 to 373b2c9 Compare November 15, 2024 19:13
avoid tooManyRequests pulling images simulteanously in CI
@dav1do dav1do mentioned this pull request Nov 15, 2024
@dav1do dav1do changed the title chore: rework build chore: rework build and integration tests Nov 15, 2024
@dav1do dav1do requested a review from nathanielc November 15, 2024 22:46
@dav1do dav1do merged commit 20a410d into main Nov 19, 2024
5 checks passed
@dav1do dav1do deleted the chore/rework-build branch November 19, 2024 19:52
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