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

Initial test around stampy.ts #276

Merged
merged 57 commits into from
Oct 7, 2023
Merged

Conversation

jrhender
Copy link
Collaborator

@jrhender jrhender commented Jul 9, 2023

This PR uses the miniflare jest env (https://miniflare.dev/testing/jest) to run a test on code that has a dependency on CF workers, in this case server-utils/stampy.ts.

The new test, in stampy.spect.ts, relies on a data which is stored in the repository in the mocks directory.
The data can be refreshed manually by running npm run refresh-test-data.

The only app code change (other than new exports) a cut-paste of Coda URL generation from stampy.ts to a separate file. Other wise, compilation of the refresh coda data script fails due to missing CF vars (e.g. CODA_TOKEN).

TODO:

  • ensure that latest code is being tested (as it seems to be the built CF worker file that is executed)
  • add script for generating test data
  • write README on how to use the script
  • parameterize the test to run for each result of the mock data
  • share the same question ids between the script and the test (was too challenging to get the compilation to work)
  • use questionId 0 for test
  • manually run app to do basic sanity test (e.g. can app still get question details from Coda)

Relates to issue #191

This wasn't the case before running the tests in miniflare jest env.
So maybe it is related to miniflare somehow
tsconfig is to allow for isolated modules
can't compile stampy.ts without CF env available
Not needed for now
Trying to avoid merge conflicts
@jrhender jrhender changed the title Miniflare jest env Initial test around stampy.ts Jul 9, 2023
@jrhender jrhender marked this pull request as ready for review July 9, 2023 20:42
@jrhender jrhender marked this pull request as ready for review September 2, 2023 10:54
@jrhender
Copy link
Collaborator Author

jrhender commented Sep 2, 2023

@Aprillion I think all of the comments from your first review have been addressed. Please give the PR another review when you have the opportunity (no rush at all 😄 ).

@jrhender
Copy link
Collaborator Author

@Aprillion The PR is ready for review again when you have time. No rush.

Aprillion
Aprillion previously approved these changes Sep 21, 2023
@jrhender
Copy link
Collaborator Author

jrhender commented Oct 6, 2023

Hey @Aprillion, no rush, but just checking in here. What would you say the next steps for this PR would be? Are there additional reviews or changes required?
If it is indeed ready and it is a good time to merge, could you please do so when you have capacity? I don't currently have write access to the repo, so I can't merge it myself.

@Aprillion
Copy link
Collaborator

oh, GH actions needed some extra approval, not just PR approval.. the merge button should get enabled after the checks run I hope..

@Aprillion
Copy link
Collaborator

@jrhender one of the tests failed in https://github.com/StampyAI/stampy-ui/actions/runs/6245510948/job/17457463802?pr=276#step:3:139 FAIL app/server-utils/stampy.spec.ts -> loadQuestionDetail › can load question 0, can you please check?

@jrhender
Copy link
Collaborator Author

jrhender commented Oct 6, 2023

@Aprillion could you approval the workflow again when you have a moment please?
I think the issue was that the feature branch (miniflare-jest-env) didn't have the last changes so it was trying to make a request to Coda that probably wasn't available due to some change in Coda. I thought about merging in master before posting my earlier message but I was too lazy to do so 😓 .
At any rate, the CI is now passing on my GH fork (https://github.com/jrhender/stampy-ui/actions/runs/6435300979/job/17476259392?pr=2), so I'm pretty sure it will pass now in this PR.

@jrhender
Copy link
Collaborator Author

jrhender commented Oct 6, 2023

Whoops, sorry, I guess it needs to be reviewed again as well as I updated the cached coda data in the last commit: 30845a9

Copy link
Collaborator

@Aprillion Aprillion left a comment

Choose a reason for hiding this comment

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

the tests are green for me locally too, no idea why it fails in the GH actions of the main repo 🤔

let's try to merge and see if the tests will start working for next PR - and if now, we will disable and investigate later 🤷

@Aprillion Aprillion merged commit 73f96b1 into StampyAI:master Oct 7, 2023
1 check failed
@Aprillion
Copy link
Collaborator

@jrhender jrhender deleted the miniflare-jest-env branch January 17, 2024 21:46
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