Justintime50 / homebrew-releaser

Release scripts, binaries, and executables directly to Homebrew via GitHub Actions.
MIT License
45 stars 8 forks source link

Setting `update_readme_table: true` produces a garbled table #19

Closed tnahs closed 2 years ago

tnahs commented 2 years ago

GitHub job: https://github.com/tnahs/test-readstor/actions/runs/3039603276/jobs/4894751016 YAML: https://github.com/tnahs/test-readstor/blob/0.1.28/.github/workflows/release.yml#L53-L67 Target Repo: https://github.com/tnahs/homebrew-test-formulas

| Project                                                 | Description | Install                      |
| ------------------------------------------------------- | ----------- | ---------------------------- |
| [test-readstor](https://github.com/tnahs/test-readstor) | This?       | `brew install test-readstor` |
#| Project                                                 | Description | Install                      |
| ------------------------------------------------------- | ----------- | ---------------------------- |
| [test-readstor](https://github.com/tnahs/test-readstor) | This?       | `brew install test-readstor` |
 | Project                                                 | Description | Install                      |
| ------------------------------------------------------- | ----------- | ---------------------------- |
| [test-readstor](https://github.com/tnahs/test-readstor) | This?       | `brew install test-readstor` |
t| Project                                                 | Description | Install                      |
| ------------------------------------------------------- | ----------- | ---------------------------- |
| [test-readstor](https://github.com/tnahs/test-readstor) | This?       | `brew install test-readstor` |
e| Project                                                 | Description | Install                      |
| ------------------------------------------------------- | ----------- | ---------------------------- |
| [test-readstor](https://github.com/tnahs/test-readstor) | This?       | `brew install test-readstor` |
s| Project                                                 | Description | Install                      |
| ------------------------------------------------------- | ----------- | ---------------------------- |
| [test-readstor](https://github.com/tnahs/test-readstor) | This?       | `brew install test-readstor` |
t| Project                                                 | Description | Install                      |
| ------------------------------------------------------- | ----------- | ---------------------------- |
| [test-readstor](https://github.com/tnahs/test-readstor) | This?       | `brew install test-readstor` |
-| Project                                                 | Description | Install                      |
| ------------------------------------------------------- | ----------- | ---------------------------- |
| [test-readstor](https://github.com/tnahs/test-readstor) | This?       | `brew install test-readstor` |
h| Project                                                 | Description | Install                      |
| ------------------------------------------------------- | ----------- | ---------------------------- |
| [test-readstor](https://github.com/tnahs/test-readstor) | This?       | `brew install test-readstor` |
o| Project                                                 | Description | Install                      |
| ------------------------------------------------------- | ----------- | ---------------------------- |
| [test-readstor](https://github.com/tnahs/test-readstor) | This?       | `brew install test-readstor` |
m| Project                                                 | Description | Install                      |
| ------------------------------------------------------- | ----------- | ---------------------------- |
| [test-readstor](https://github.com/tnahs/test-readstor) | This?       | `brew install test-readstor` |
e| Project                                                 | Description | Install                      |
| ------------------------------------------------------- | ----------- | ---------------------------- |
| [test-readstor](https://github.com/tnahs/test-readstor) | This?       | `brew install test-readstor` |
b| Project                                                 | Description | Install                      |
| ------------------------------------------------------- | ----------- | ---------------------------- |
| [test-readstor](https://github.com/tnahs/test-readstor) | This?       | `brew install test-readstor` |
r| Project                                                 | Description | Install                      |
| ------------------------------------------------------- | ----------- | ---------------------------- |
| [test-readstor](https://github.com/tnahs/test-readstor) | This?       | `brew install test-readstor` |
e| Project                                                 | Description | Install                      |
| ------------------------------------------------------- | ----------- | ---------------------------- |
| [test-readstor](https://github.com/tnahs/test-readstor) | This?       | `brew install test-readstor` |
w| Project                                                 | Description | Install                      |
| ------------------------------------------------------- | ----------- | ---------------------------- |
| [test-readstor](https://github.com/tnahs/test-readstor) | This?       | `brew install test-readstor` |
-| Project                                                 | Description | Install                      |
| ------------------------------------------------------- | ----------- | ---------------------------- |
| [test-readstor](https://github.com/tnahs/test-readstor) | This?       | `brew install test-readstor` |
f| Project                                                 | Description | Install                      |
| ------------------------------------------------------- | ----------- | ---------------------------- |
| [test-readstor](https://github.com/tnahs/test-readstor) | This?       | `brew install test-readstor` |
o| Project                                                 | Description | Install                      |
| ------------------------------------------------------- | ----------- | ---------------------------- |
| [test-readstor](https://github.com/tnahs/test-readstor) | This?       | `brew install test-readstor` |
r| Project                                                 | Description | Install                      |
| ------------------------------------------------------- | ----------- | ---------------------------- |
| [test-readstor](https://github.com/tnahs/test-readstor) | This?       | `brew install test-readstor` |
m| Project                                                 | Description | Install                      |
| ------------------------------------------------------- | ----------- | ---------------------------- |
| [test-readstor](https://github.com/tnahs/test-readstor) | This?       | `brew install test-readstor` |
u| Project                                                 | Description | Install                      |
| ------------------------------------------------------- | ----------- | ---------------------------- |
| [test-readstor](https://github.com/tnahs/test-readstor) | This?       | `brew install test-readstor` |
l| Project                                                 | Description | Install                      |
| ------------------------------------------------------- | ----------- | ---------------------------- |
| [test-readstor](https://github.com/tnahs/test-readstor) | This?       | `brew install test-readstor` |
a| Project                                                 | Description | Install                      |
| ------------------------------------------------------- | ----------- | ---------------------------- |
| [test-readstor](https://github.com/tnahs/test-readstor) | This?       | `brew install test-readstor` |
s| Project                                                 | Description | Install                      |
| ------------------------------------------------------- | ----------- | ---------------------------- |
| [test-readstor](https://github.com/tnahs/test-readstor) | This?       | `brew install test-readstor` |
Project Description Install
test-readstor This? brew install test-readstor
# Project Description Install
------------------------------------------------------- ----------- ----------------------------
test-readstor This? brew install test-readstor
Project Description Install
------------------------------------------------------- ----------- ----------------------------
test-readstor This? brew install test-readstor
t Project Description Install
------------------------------------------------------- ----------- ----------------------------
test-readstor This? brew install test-readstor
e Project Description Install
------------------------------------------------------- ----------- ----------------------------
test-readstor This? brew install test-readstor
s Project Description Install
------------------------------------------------------- ----------- ----------------------------
test-readstor This? brew install test-readstor
t Project Description Install
------------------------------------------------------- ----------- ----------------------------
test-readstor This? brew install test-readstor
- Project Description Install
------------------------------------------------------- ----------- ----------------------------
test-readstor This? brew install test-readstor
h Project Description Install
------------------------------------------------------- ----------- ----------------------------
test-readstor This? brew install test-readstor
o Project Description Install
------------------------------------------------------- ----------- ----------------------------
test-readstor This? brew install test-readstor
m Project Description Install
------------------------------------------------------- ----------- ----------------------------
test-readstor This? brew install test-readstor
e Project Description Install
------------------------------------------------------- ----------- ----------------------------
test-readstor This? brew install test-readstor
b Project Description Install
------------------------------------------------------- ----------- ----------------------------
test-readstor This? brew install test-readstor
r Project Description Install
------------------------------------------------------- ----------- ----------------------------
test-readstor This? brew install test-readstor
e Project Description Install
------------------------------------------------------- ----------- ----------------------------
test-readstor This? brew install test-readstor
w Project Description Install
------------------------------------------------------- ----------- ----------------------------
test-readstor This? brew install test-readstor
- Project Description Install
------------------------------------------------------- ----------- ----------------------------
test-readstor This? brew install test-readstor
f Project Description Install
------------------------------------------------------- ----------- ----------------------------
test-readstor This? brew install test-readstor
o Project Description Install
------------------------------------------------------- ----------- ----------------------------
test-readstor This? brew install test-readstor
r Project Description Install
------------------------------------------------------- ----------- ----------------------------
test-readstor This? brew install test-readstor
m Project Description Install
------------------------------------------------------- ----------- ----------------------------
test-readstor This? brew install test-readstor
u Project Description Install
------------------------------------------------------- ----------- ----------------------------
test-readstor This? brew install test-readstor
l Project Description Install
------------------------------------------------------- ----------- ----------------------------
test-readstor This? brew install test-readstor
a Project Description Install
------------------------------------------------------- ----------- ----------------------------
test-readstor This? brew install test-readstor
s Project Description Install
------------------------------------------------------- ----------- ----------------------------
test-readstor This? brew install test-readstor
Justintime50 commented 2 years ago

Huh... this one is interesting:

  1. One of the errors in the output states Could not find start/end tags for project table in README, seems like when it can't find the start/end tags it blows up. You'll need to add the start/end tags
  2. You don't appear to be setting the update_readme_table boolean correct? It should be skipping the README table generation completely, or did you add that in?
  3. The bug itself seems to be splitting the word into a separate entry on each which is something I'll need to address
Justintime50 commented 2 years ago

I've pushed another update that should address this. Whenever an old table cannot be found (see README instructions for details on how that works), it will now skip generating and attempting to replace the old one. I had an if statement missing in there.

That still leaves two questions unanswered that I could use your help determining on your next run:

  1. Does the README try to update even when the boolean to do so isn't specified (shouldn't be the case)
  2. If you do want to be updating your README tables, does it still blow up like above when all the settings are setup correctly (in your case, you needed to add the start and end tags so this knows where your README is, see instructions for details). I'm curious if it blew up like the above simply due to the missing check and will now behave correctly.

If all looks well, I'll close this one, but will wait for you to let me know how it goes. Thanks for your patience while fixing some of these edge cases!

tnahs commented 2 years ago

Thanks so much for looking into this! I'll get back to you later today, I won't have a chance to test until then.

tnahs commented 2 years ago

Oh I completely missed the tags in the instructions. Okay, I tried a few things but it seems to not be working at all now.

update_readme_table: true & missing tags -> no table and no garbled table.

update_readme_table: false -> no table and no garbled table.

update_readme_table: true & with tags -> no table and no garbled table. Using:

# homebrew-test-formula

<!-- project_table_start -->

<!-- project_table_end -->

2022-09-13 06:05:43,701 - ERROR - Could not find start/end tags for project table in README. https://github.com/tnahs/test-readstor/actions/runs/3042708794/jobs/4901178392 https://github.com/tnahs/test-readstor/blob/0.1.34/.github/workflows/release.yml#L66

And just in case:

update_readme_table: true & with tags -> no table and no garbled table. Using:

# homebrew-test-formula

<!-- project_table_start -->
TABLE HERE
<!-- project_table_end -->

2022-09-13 06:10:19,112 - ERROR - Could not find start/end tags for project table in README. https://github.com/tnahs/test-readstor/actions/runs/3042732891/jobs/4901224862 https://github.com/tnahs/test-readstor/blob/0.1.35/.github/workflows/release.yml#L66

I checked your formulas repo and it seems to be doing something very similar. Not sure what's going on.

Justintime50 commented 2 years ago

Ok, I was able to run your workflow locally and got the same error, but after cloning your repo, I'm still not finding that you added the tags to your README in any of your recent commits. Without the tag comments in the README, the releaser won't be able to update the table contents. The README tags need to be in the default branch on GitHub for this to take effect. If you make that correction then you should be golden.

EDIT: I actually misspoke and was looking in the wrong place. Still investigating.

tnahs commented 2 years ago

Hmm... I'm pretty sure I did, unless I'm putting it in the wrong place? This is 3 commits ago: https://github.com/tnahs/homebrew-test-formulas/commit/408097d85b2d0e468abd41f109f79a7fd80d729c

Justintime50 commented 2 years ago

Yeah, just realized I was looking in the wrong place 😭 Your setup looks correct which makes this even more interesting, especially because my own projects can properly generate the table so I know it works.

Justintime50 commented 2 years ago

Heh, I figured it out. This should be fixed via https://github.com/Justintime50/homebrew-releaser/commit/8e5c3b17cf1cce724843922aba0ad6a38d84e5e6 and will be released within the next ~10 mins.

Thanks again for reporting this! Let me know if you have any other troubles.

tnahs commented 2 years ago

Hey! Np! 😄

I just ran the action and it worked great except I did notice that it removes the table start tag <!-- project_table_start -->: https://github.com/tnahs/homebrew-test-formulas/commit/c16dc9af9c6179a43b9c1c369ac7659612b82c1c

I ran it once, it removed it. I added it back and then re-ran the action and it removed it again. You can see the history here: https://github.com/tnahs/homebrew-test-formulas/commits/main/README.md

Justintime50 commented 2 years ago

Awe geez. I'm embarrassed heh. This is fixed via https://github.com/Justintime50/homebrew-releaser/commit/24ad8bc3364aee36c33db0e6057aa5869035f00d.

Unfortunately the README module of this action is the only thing without comprehensive unit tests due to the difficult nature of mocking a git repo. Sounds worthwhile for me to revisit.

tnahs commented 2 years ago

Cool thanks! I just ran a bunch of tests and just wanted to document them here in case this wasn't the functionality you wanted and if you did, it might help the next person. :smile:

I tried three different configurations and it seems like having some text between the two tags is necessary. The documentation shows text between the two tags but I wasn't sure if that was optional or not.

The following examples represent everything inside the repo's README.md file.

Suceeded:

# repo-name

<!-- project_table_start -->
test
<!-- project_table_end -->

Failed:

# repo-name

<!-- project_table_start -->

<!-- project_table_end -->

Failed:

# repo-name

<!-- project_table_start -->
<!-- project_table_end -->
Justintime50 commented 2 years ago

Goodness me. 😂 Cool, I'll see about fixing this over the weekend if I have time. I'll also revisit comprehensive unit tests for this module as it could obviously use an overhaul.

Justintime50 commented 2 years ago

This should now be fixed! I reworked how it replaces the table (should now be inclusive of the start/end tags when they both exist meaning it'll include everything between tags (meaning real content or null content) since the tags are now the bookends and not the content (can't replace nothing). Check it out via https://github.com/Justintime50/homebrew-releaser/commit/3905f32964fe50290ec037db7a25477848bd241d.

Thanks again for calling me out on these items. It really helps to improve the action. You have been INSTRUMENTAL in fixing half a dozen bugs here and for incentivizing me to come back and give this app some TLC and get the test coverage to 100%. There may still be some small end-to-end details missed in the test suite due to how GitHub Actions interacts with the app, but the app itself should now be pretty solid thanks to your help!

tnahs commented 2 years ago

Hey no problem! Glad I wasn't a bother. It's a fantastic action, so I'm glad I can contribute somehow! 😄