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

Dummy app test setup improvements #57

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

codyrobbins
Copy link
Contributor

@codyrobbins codyrobbins commented Apr 16, 2024

I struggled a bit getting the dummy app test suite running locally. The commands in the GitHub Actions config file didn’t work for me for a variety of reasons, probably due to Bundler version and configuration differences and having asdf in the mix. Once I got it working on my machine, I moved the commands into bin/dummy-app-setup and modified the GitHub Actions workflow to call this script.

I think this dummy app setup script is more general and can now be called locally on anyone’s machine to prepare the dummy app for running the test suite. I changed the directory structure created slightly to ensure that the gem’s development dependencies, the rails gem installation used to call rails new, and the dummy app gem dependencies are all firewalled off from each other. This doesn’t matter when running via GitHub Actions, but was a big part of the problem when trying to run locally. The modified directory structure also allows dummy apps for multiple versions of Rails to coexist in spec/tmp/dummy_app-rails-VERSION/dummy_app.

It’s possible these changes might be depending on quirks of my own local development environment, though, so I’m not sure if this will run correctly on GitHub Actions or your own machine. But if the kinks can be worked out I think this will improve the development experience for anyone else contributing!

@codyrobbins codyrobbins force-pushed the dummy-app-test-setup-improvements branch from 4af59cd to d1283ae Compare April 16, 2024 16:27
… suite locally and changed the GitHub Actions workflow to leverage the new setup script.
@codyrobbins codyrobbins force-pushed the dummy-app-test-setup-improvements branch from d1283ae to 6078cbf Compare April 16, 2024 17:06
@codyrobbins
Copy link
Contributor Author

codyrobbins commented Apr 16, 2024

Update: It looks like this works on GitHub Actions (after a few minor fixes)!

@abevoelker
Copy link
Owner

I appreciate the effort on this! 🎉 🚀 And I'm sorry to hear of your struggles to run the suite locally - this is something that definitely should've been documented!

I cherry-picked your README change and edited it to reflect the current reality of testing: 166f0ba

As mentioned in that change, to test the dummy app locally I use https://github.com/nektos/act which runs the GitHub workflow/action locally using Docker.

This used to be really simple like 6 months ago; I could just do

$ act -j test

And it would work fine. But I recently updated to the latest act to resolve your PR and all heck broke loose. I made a couple changes to the GitHub workflow YAML to fix that, and now the act command to use is a bit more unwieldy because its built-in caching stuff seems unstable, and an alternate Docker image is required due to user permissions:

$ act -W .github/workflows/test.yml -P ubuntu-latest=ghcr.io/catthehacker/ubuntu:act-latest --no-cache-server

If you only want to run against certain versions of Ruby or Rails you can use the --matrix flag:

$ act -W .github/workflows/test.yml -P ubuntu-latest=ghcr.io/catthehacker/ubuntu:act-latest --no-cache-server --matrix ruby-version:3.2 --matrix rails-version:7

That said, I see the value in your script and would be happy to accept your PR with a couple changes!

  1. The most recent GitHub Action run of the PR isn't actually running the dummy app test suite AFAICT - if you expand the "Run Rails dummy app tests" dropdown entry you'll see it's just listing rails new usage so something is funky there:

    Screenshot from 2024-04-27 16-23-57

    So, get that working to run the tests

  2. Sync up your script's logic with the minor changes I had to make to the GitHub workflow YAML over the past couple days: b2e195b...master

  3. Rework your README blurb into the updated wording (code changed per #2)

If you need to re-push / force push to re-run GitHub actions/workflows that's no problem, just let me know when you think it's ready

Thanks!

@abevoelker
Copy link
Owner

(Copying and pasting this to all open issues/PRs:)

Hey all, per #64 I unfortunately won't have much time for the foreseeable future to maintain devise-passwordless to fix the open bugs and work on new features. I'm not abandoning this project, but due to some life issues it's just at the bottom of my priority list for now.

Anyone who wants to step up and be a maintainer to shepherd the project forward would be welcomed! I just ask that you've opened a PR, or written an issue, or can otherwise demonstrate some familiarity/competence with the project. You can reply to #64 or message me privately (through email or socials since GitHub doesn't have DMs) if interested. Thank you ✌️

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