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

feature: client linting and CI #195

Merged
merged 21 commits into from
Jan 26, 2024
Merged

Conversation

joshuagraber
Copy link
Contributor

@joshuagraber joshuagraber commented Jan 25, 2024

Will take care of #165 and #191 when merged to main.

No manual testing needed, just observe successful pull_request workflow run on this PR.

@joshuagraber joshuagraber marked this pull request as draft January 25, 2024 14:36
@joshuagraber joshuagraber marked this pull request as ready for review January 25, 2024 14:45
@joshuagraber
Copy link
Contributor Author

Hmmm @mbodeantor client tests are matching snapshots locally but not in CI. 🤔 any ideas on why that might be? I'm a bit flummoxed.

@mbodeantor
Copy link
Contributor

@joshuagraber I'm not familiar with client testing, where is the source of the snapshots set?

@joshuagraber
Copy link
Contributor Author

I'm not familiar with client testing, where is the source of the snapshots set?

Snapshots are set manually by passing a flag in command line. Manually because devs should inspect failed tests first to make sure nothing unintended is breaking. When the update flag is passed, vitest renders the component as vue would, and prints the resulting markup to the snapshot file.

I'm asking more about if you know why CI would behave differently than a local machine environment, because it makes no sense to me that we'd get different results from the same files.

Still working on it and Googling some things, just wanted to check and see if you had any ideas top of mind.

@joshuagraber
Copy link
Contributor Author

cc @mbodeantor ^

@mbodeantor
Copy link
Contributor

@joshuagraber What does setting manually in the command line correspond to in the CI config? If the snapshot match is working locally, it seems like the CI script is not pulling the snapshot in correctly

@joshuagraber
Copy link
Contributor Author

joshuagraber commented Jan 25, 2024

it seems like the CI script is not pulling the snapshot in correctly

@mbodeantor No, it's getting the snapshot files as expected (those are static and committed to version control). The problem is with the test evaluation itself. The tests render the html from whatever component we're testing then check against the static snapshot file. The snapshot file should not change unless the associated component files have changed. So it must be something in the result of the .html() method on our test wrappers that is different between CI and local CLI. Really bizarre behavior.

@joshuagraber
Copy link
Contributor Author

@mbodeantor G2g here. I looked a bit more closely at exactly how the snapshots were mismatched here, and the dates weren't lining up (expected because GH Actions defaults to UTC, so some dates were pushed into the future). I added a time zone setter to the test script to match with US EST, and that now solves it.

If we work with any team members drastically ahead of or behind the U.S. at some point, we may need to amend this, but I think it's fine for now.

@mbodeantor mbodeantor merged commit 29d7f65 into dev Jan 26, 2024
8 checks passed
@EvilDrPurple EvilDrPurple deleted the feature/client-linting-and-ci branch June 18, 2024 23:25
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