Fix flaky unit test: linked_module_resolver_test

[c0d1ngm0nk3y]

Summary

The unit test linked_module_resolver_test was flaky.

Use Cases

Checklist

• [X] I have viewed, signed, and submitted the Contributor License Agreement.
• [ ] I have linked issue(s) that this PR should close using keywords or the Github UI (See docs)
• [ ] I have added an integration test, if necessary.
• [X] I have reviewed the styleguide for guidance on my code quality.
• [X] I'm happy with the commit history on this PR (I have rebased/squashed as needed).

[c0d1ngm0nk3y]

@ryanmoran Could you have a look?

[c0d1ngm0nk3y]

Is there anything wrong/missing with this pr? It looks like the flake is already in main for month.

[pacostas]

Is there anything wrong/missing with this pr? It looks like the flake is already in main for month.

@c0d1ngm0nk3y We were looking at the issue with @mhdawson. We concluded that it is preferred to keep the structure of the test data as the rationale of the author who wrote the test, seems to be on erroring over wrong permissions, so we think its good to follow the same pattern instead of testing it over something new. So instead of creating a file and erroring over that, you can change the permissions of the directory where the module is trying to create the subsequent directories.

This would mean, that for the first flaky test on line 132 you can do

            context("when the destination cannot be scaffolded", func() {
                it.Before(func() {
                    // Module 1 to 4 are written under sub-dir
                    Expect(os.Mkdir(filepath.Join(layerPath, "sub-dir"), 0400)).To(Succeed())
                    // Module 5 is written under layerPath
                    Expect(os.Chmod(layerPath, 0400)).To(Succeed())
                })

                it("returns an error", func() {
                    err := resolver.Resolve(filepath.Join(workspace, "package-lock.json"), filepath.Join(layerPath, "sub-dir"))
                    Expect(err).To(MatchError(ContainSubstring("failed to setup linked module directory scaffolding")))
                })
                it.After(func() {
                    Expect(os.Chmod(filepath.Join(layerPath), 0700)).To(Succeed())
                })
            })

and for the second flaky test on line 216 you can do

            context("when the destination cannot be scaffolded", func() {
                it.Before(func() {
                    // Module 1 to 4 are written under sub-dir
                    Expect(os.Mkdir(filepath.Join(otherLayerPath, "sub-dir"), 0400)).To(Succeed())
                    // Module 5 is written under otherLayerPath
                    Expect(os.Chmod(otherLayerPath, 0400)).To(Succeed())
                })

                it("returns an error", func() {
                    err := resolver.Resolve(filepath.Join(workspace, "package-lock.json"), filepath.Join(otherLayerPath, "sub-dir"))
                    Expect(err).To(MatchError(ContainSubstring("failed to setup linked module directory scaffolding")))
                })

                it.After(func() {
                    Expect(os.Chmod(filepath.Join(otherLayerPath), 0700)).To(Succeed())
                })
            })

[c0d1ngm0nk3y]

We concluded that it is preferred to keep the structure of the test data as the rationale of the author who wrote the test, seems to be on erroring over wrong permissions, so we think its good to follow the same pattern instead of testing it over something new.

But the test is indeterministic because of the way golang loops over dicts. The author of this test clearly wanted to target a very specific error and the very same error will be raised regardless on how the error is provoked.

Any additional information should be reflected in some form in the test imo.

[c0d1ngm0nk3y]

I changed the fix to address the underlying problem. The testdata containes several modules which are looped over in a non-deterministic order. Not ideal for an error test. I reduced the testdata for those cases, making tests more readable imho.

@paketo-buildpacks/nodejs-maintainers Could you have another look?