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

enhancement: Improve message when missing resource for array fields #3733

Conversation

zhephyn
Copy link
Contributor

@zhephyn zhephyn commented Mar 11, 2025

Description

Improves message shown when missing a resource such that when the resource is of the type array, we display it with an array --array flag. This is a newer cleaner PR unlike the one from yesterday.

Fixes #3713

Checklist:

  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • I have added tests that prove my fix is effective or that my feature works

Copy link

codeclimate bot commented Mar 11, 2025

Code Climate has analyzed commit c687715 and detected 0 issues on this pull request.

View more on Code Climate.

@zhephyn
Copy link
Contributor Author

zhephyn commented Mar 11, 2025

Hey @Paul-Bob . Just got done with this. Tagging you here because i can't see the "request for review" button.

PS: I have some questions regarding how Avo does testing so that in the future i don't run into the "tests are failing on my local machine but passing on main" issue from yesterday. Kindly get back to me when you have some time regarding this.

As for this PR, if it gets merged, I'll pick up another issue for assignment ASAP.
Thanks for your patience!

@Paul-Bob
Copy link
Contributor

Hey @Paul-Bob . Just got done with this. Tagging you here because i can't see the "request for review" button.

Hi @zhephyn thanks for submitting this contribution.

PS: I have some questions regarding how Avo does testing so that in the future i don't run into the "tests are failing on my local machine but passing on main" issue from yesterday. Kindly get back to me when you have some time regarding this.

Do you have any specific questions? What happened yesterday was that your PR included a lot of unrelated changes, which caused some tests to fail. Most likely, your local branch was outdated when you created it. To prevent this in the future, always run git pull on main before starting a new branch.

As for this PR, if it gets merged, I'll pick up another issue for assignment ASAP. Thanks for your patience!

Sounds good! I'll review this one today

@zhephyn
Copy link
Contributor Author

zhephyn commented Mar 11, 2025

Yeah partly the issue was that the branch was updated, but even when working on the branch for this new PR, after pulling the latest changes, when i ran some tests, some of them were still failing. I spent the entire part of today's morning trying to figure out why this happens, even considered running the tests in a docker container.

What option do you guys recommend when running the tests locally which mirrors atleast what the github CI does when running the tests? Is running the tests in a docker container a good option?

To give an example, the missing_resource_message method is flagged by rubocop stating that the line below is too long,
"def missing_resource_message(model_class, field_name, field)"

However, on pushing the code, Github CI didn't flag this and the check passed. It's this kind of thing that is frustrating.
Another example, i spent some time debugging why the has_one test in the model_missing_resource_spec file was failing(still currently is on my local machine), only for the github CI to show me that the test is passing.

I don't know if there is an option/gem i can run locally that mirrors exactly how the github CI does its checks so that i don't run into these discrepancies.
Otherwise, thanks again for your patience. Incase you need pictures to explain better, kindly lmk.
PS: Sorry if my ramblings became too long.

@Paul-Bob
Copy link
Contributor

Yeah partly the issue was that the branch was updated, but even when working on the branch for this new PR, after pulling the latest changes, when i ran some tests, some of them were still failing. I spent the entire part of today's morning trying to figure out why this happens, even considered running the tests in a docker container.

What option do you guys recommend when running the tests locally which mirrors atleast what the github CI does when running the tests? Is running the tests in a docker container a good option?

I'm sorry you had to go through that, I know how frustrating it can be.

I just pushed a commit that addresses some local testing issues. Could you pull the latest changes and check if the tests are still failing on your end? If they are, please share the error messages and test details so we can debug together.

Since local environments vary across developers, test behavior can sometimes be inconsistent. If the issue persists even after syncing with main, let me know so we can investigate further.

I also came across act, which lets you run GitHub Actions locally. I haven’t tested it yet, but it might help improve the contributions workflow. I'll give it a try soon, let me know if you do as well!

To give an example, the missing_resource_message method is flagged by rubocop stating that the line below is too long,
"def missing_resource_message(model_class, field_name, field)"

However, on pushing the code, Github CI didn't flag this and the check passed. It's this kind of thing that is frustrating.
Another example, i spent some time debugging why the has_one test in the model_missing_resource_spec file was failing(still currently is on my local machine), only for the github CI to show me that the test is passing.

We use StandardRB, and the action runs via this repository.

Did you run rubocop locally to verify the "missing_resource_message" method length issue?

For the failing has_one test in model_missing_resource_spec, if it's passing on GitHub CI but failing locally, there might be an environment-related difference. Could you share the exact error message and details on how you're running the tests?

I don't know if there is an option/gem i can run locally that mirrors exactly how the github CI does its checks so that i don't run into these discrepancies.

If you’re looking for a way to mirror GitHub CI checks locally, act could be useful. It lets you run GitHub Actions on your machine. Let me know if you try it out!


This PR looks good, thanks for submitting it, I'll just commit some tweaks and merge it!

@Paul-Bob Paul-Bob added the Enhancement Not necessarily a feature, but something has improved label Mar 11, 2025
@Paul-Bob Paul-Bob merged commit 3e93427 into avo-hq:main Mar 11, 2025
20 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Enhancement Not necessarily a feature, but something has improved
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Improve message when is missing array resource for array fields
2 participants