Skip to content
This repository has been archived by the owner on Aug 13, 2024. It is now read-only.

feat: adding utilities for cards local generation & testing #34

Merged
merged 9 commits into from
Apr 26, 2023

Conversation

nightknighto
Copy link
Contributor

@nightknighto nightknighto commented Apr 19, 2023

What type of PR is this? (check all applicable)

  • πŸ• Feature
  • πŸ› Bug Fix
  • πŸ“ Documentation Update
  • 🎨 Style
  • πŸ§‘β€πŸ’» Code Refactor
  • πŸ”₯ Performance Improvements
  • βœ… Test
  • πŸ€– Build
  • πŸ” CI
  • πŸ“¦ Chore (Release)
  • ⏩ Revert

Description

  • Adds local development script for generating images from a given testing array of usernames, without interacting with the cloud.
  • Chose separate scripts to not mess with the main flow of the app used for production and interacting with the cloud.
  • Run npm run local-dev:usercards to run the script and generate the images for local testing.

Related Tickets & Documents

fixes #31 & #26

Mobile & Desktop Screenshots/Recordings

Added tests?

  • πŸ‘ yes
  • πŸ™… no, because they aren't needed
  • πŸ™‹ no, because I need help

Added to documentation?

  • πŸ“œ README.md
  • πŸ““ docs.opensauced.pizza
  • πŸ• dev.to/opensauced
  • πŸ“• storybook
  • πŸ™… no documentation needed

[optional] Are there any post-deployment tasks we need to perform?

[optional] What gif best describes this PR or how it makes you feel?

Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

PR Compliance Checks

Thank you for your Pull Request! We have run several checks on this pull request in order to make sure it's suitable for merging into this project. The results are listed in the following section.

Watched Files

This pull request modifies specific files that require careful review by the maintainers.

Files Matched

  • package.json

Copy link
Contributor

@0-vortex 0-vortex left a comment

Choose a reason for hiding this comment

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

Couple of small changes required for base nestjs testing practices. How does testing actually work with this new workflow, can you add it to the testing section of the Readme? πŸ•

.gitignore Outdated Show resolved Hide resolved
local-dev/UserCards.ts Outdated Show resolved Hide resolved
local-dev/UserCards.ts Outdated Show resolved Hide resolved
@nightknighto
Copy link
Contributor Author

nightknighto commented Apr 20, 2023

@0-vortex Hmm, well it was meant to be a form of manual standalone testing to check the appearance not automated testing.

Didn't have much luck getting jest to acknowledge the e2e test files in /test as well.

@0-vortex
Copy link
Contributor

@0-vortex Hmm, well it was meant to be a form of manual standalone testing to check the appearance not automated testing.

Ok, other good folder ideas for the static output:

  • dist
  • out
  • coverage

@0-vortex
Copy link
Contributor

@0-vortex Hmm, well it was meant to be a form of manual standalone testing to check the appearance not automated testing.

A fix for avoiding the automated testing failures is to return status code 0 on the base test command and implement our own testing command as a separate script - that way we could keep the testing boilerplate code inside the test folder while avoiding CI issues πŸ•

@nightknighto
Copy link
Contributor Author

@0-vortex Hmm, well it was meant to be a form of manual standalone testing to check the appearance not automated testing.

A fix for avoiding the automated testing failures is to return status code 0 on the base test command and implement our own testing command as a separate script - that way we could keep the testing boilerplate code inside the test folder while avoiding CI issues πŸ•

Yup added a separate command for running the generation file.

Copy link
Contributor

@0-vortex 0-vortex left a comment

Choose a reason for hiding this comment

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

Managed to give this a spin locally and imagined making a user card version 2, the resulting SVG is matching the generated image and can be used to pixel perfect the design without very much trial and error, being very close to local development with a NextJS based frontend; because of the pros and cons of vercel/satori the workflow proposed in this PR is close to best of what we can get.

The paths provided by the svg can be inspected in devtools and provide x/y coordinates by default, making it easy to debug and prototype new cards:

Screenshot 2023-04-21 at 12 14 23

Have a couple more minor changes to suggest to make this perfect IMHO:

Screenshot 2023-04-21 at 12 15 02

Let me know if you find this reasonable ❀️

cc @brandonroberts think this PR is solving the blocker and making development bearable, what are your thoughts?

@nightknighto
Copy link
Contributor Author

@0-vortex The reason I made UserCard.ts and not a unified file for all local dev is so that each different social card can be generated and tested on its own (highlight cards coming up next). This saves time by not generating other unneeded social cards when testing a specific one and making code more manageable. What do you think?

@0-vortex
Copy link
Contributor

@0-vortex The reason I made UserCard.ts and not a unified file for all local dev is so that each different social card can be generated and tested on its own (highlight cards coming up next). This saves time by not generating other unneeded social cards when testing a specific one and making code more manageable. What do you think?

future social cards could pose some unknown requirements for which we could only make assumptions now; don't think the way we split the code should affect the way we run the testing commands or rather, limit ourselves to optimising those via code only; an example here is we could keep it with whatever name as long as it's in test folder, then split the test runner from the social cards or implement a parameter to the testing command like npm run test:local user. In the suggested changes my issue was the "local-dev" folder holding a single file πŸ˜… πŸ•

@nightknighto
Copy link
Contributor Author

nightknighto commented Apr 21, 2023

@0-vortex Alright, no problem πŸ‘
Can you push from your repo or do I need to do those changes in mine and push?

@brandonroberts
Copy link
Contributor

I like the separate npm script command for usercards. We could make a static page with the sample usernames that fetch from the running backend also, but I think its good enough for now.

@nightknighto
Copy link
Contributor Author

@0-vortex

Copy link
Contributor

@0-vortex 0-vortex left a comment

Choose a reason for hiding this comment

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

2 small changes are still needed, test command should get renamed too πŸ•

test/local-dev/UserCards.ts Outdated Show resolved Hide resolved
@nightknighto nightknighto merged commit c5a5fec into beta Apr 26, 2023
@nightknighto nightknighto deleted the cards-local-testing branch April 26, 2023 11:34
github-actions bot pushed a commit that referenced this pull request Apr 26, 2023
## [2.1.0-beta.1](v2.0.1...v2.1.0-beta.1) (2023-04-26)

### πŸ• Features

* adding utilities for cards local generation & testing ([#34](#34)) ([c5a5fec](c5a5fec))
@github-actions
Copy link

πŸŽ‰ This PR is included in version 2.1.0-beta.1 πŸŽ‰

The release is available on:

Your semantic-release bot πŸ“¦πŸš€

github-actions bot pushed a commit that referenced this pull request Apr 26, 2023
* feat: getuserdata private

* fix: fix incorrect import

* feat: refractor & add local scripts

* feat: add local dev generation script

* last touches

* refractor: suggestions

* docs: add documentation for local dev script

* refractor: change output folder

* update local dev user command name c5a5fec
github-actions bot pushed a commit that referenced this pull request May 9, 2023
## [2.1.0](v2.0.1...v2.1.0) (2023-05-09)

### πŸ› Bug Fixes

* Overflowing language bar fix ([#38](#38)) ([3dc1a0a](3dc1a0a))

### πŸ• Features

* adding utilities for cards local generation & testing ([#34](#34)) ([c5a5fec](c5a5fec))
* Highlight Cards UI Generation (frontend-only) ([#36](#36)) ([138a847](138a847))
* highlights getting repo name & languages from pr's repo ([#41](#41)) ([1555a25](1555a25))
* higlight card upload & storage ([#39](#39)) ([b6abefa](b6abefa))
* UI adjustments ([#42](#42)) ([acbe81a](acbe81a))
@github-actions
Copy link

github-actions bot commented May 9, 2023

πŸŽ‰ This PR is included in version 2.1.0 πŸŽ‰

The release is available on:

Your semantic-release bot πŸ“¦πŸš€

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

bug: Local rendering/testing not available
3 participants