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

fix(cosmosgen): fix tests #2175

Merged
merged 3 commits into from
Mar 19, 2022
Merged

fix(cosmosgen): fix tests #2175

merged 3 commits into from
Mar 19, 2022

Conversation

ilgooz
Copy link
Member

@ilgooz ilgooz commented Mar 17, 2022

No description provided.

@ilgooz
Copy link
Member Author

ilgooz commented Mar 17, 2022

Hey @bjaanes, any ideas what is this about? https://github.com/tendermint/starport/runs/5591660944?check_suite_focus=true

@gjermundgaraba
Copy link
Contributor

@ilgooz not sure what would cause this. But I will check out the branch and do some investigation on my side

@gjermundgaraba
Copy link
Contributor

Not solving this today, will look at it again and try to figure out what is going on tomorrow.

@gjermundgaraba
Copy link
Contributor

@ilgooz I've managed to look into this and found the problem, but I am unsure why this didn't fail before. The problem is related to how the feegrant module's registration is deeper x/feegrant/module than the actual implementation itself. With the patch attached it looks for an alternative implementation path to avoid this.
I am a little under the weather due to Covid (I'm fine) so I am not 100% confident there isn't a much simpler way to do this.

diff --git a/starport/pkg/cosmosanalysis/module/module.go b/starport/pkg/cosmosanalysis/module/module.go
index 23ac8894..11b065a3 100644
--- a/starport/pkg/cosmosanalysis/module/module.go
+++ b/starport/pkg/cosmosanalysis/module/module.go
@@ -275,6 +275,16 @@ func (d *moduleDiscoverer) pkgIsFromRegisteredModule(pkg protoanalysis.Package)
                                return false, err
                        }
 
+                       // In some cases, the module registration is deeper in the folder structure
+                       if len(found) == 0 && strings.HasPrefix(rm, pkg.GoImportName) {
+                               altImplRelPath := strings.TrimPrefix(pkg.GoImportName, d.basegopath)
+                               altImplPath := filepath.Join(d.sourcePath, altImplRelPath)
+                               found, err = cosmosanalysis.DeepFindImplementation(altImplPath, methods)
+                               if err != nil {
+                                       return false, err
+                               }
+                       }
+
                        if len(found) > 0 {
                                return true, nil
                        }

@ilgooz
Copy link
Member Author

ilgooz commented Mar 19, 2022

This looks good, thank you for taking time to investigate this.

@ilgooz
Copy link
Member Author

ilgooz commented Mar 19, 2022

I've managed to look into this and found the problem, but I am unsure why this didn't fail before.

We scaffold chains with a pre generated code which comes from the template inside templates/vue (because it takes long time to generate code for all modules).

So, I added a line in cosmosgen tests to actually delete that pre generated code for a fresh code generation test. And it seemed after that cosmosgen wasn't generating code for feegrant like modules.

@ilgooz ilgooz merged commit 420d0d7 into release/v0.19.5 Mar 19, 2022
@ilgooz ilgooz deleted the fix/cosmosgen-test branch March 19, 2022 08:45
Jchicode pushed a commit to Jchicode/cli that referenced this pull request Aug 9, 2023
* fix(cosmosgen): fix tests

* fix module>proto pkg discovery

* Apply suggestions from code review
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.

None yet

2 participants