search

lando / core-next

Next generation Lando v4 runtime
https://docs.lando.dev/core/v4
GNU General Public License v3.0
3 stars 4 forks source link

yaml.safeLoad is removed in js-yaml 4 #41

Closed xurizaemon closed 11 months ago

xurizaemon commented 11 months ago

I see several open PRs (#32 #38 #39 #40) currently fail unit tests with:

  yaml
    #Yaml
      ✔ should return a Yaml instance with correct default options
    #load
ERROR ==> Problem parsing /tmp/config1.yml with Function yaml.safeLoad is removed in js-yaml 4. Use yaml.load instead, which is now safe by default. 
      4) should return data from a YAML file as an Object
      ✔ should throw an error when file does not exist
    #dump
      5) should create the directory for the file if it does not exist
      6) should write a valid YAML file to disk for the object
      7) should return the name of the file

It may be those PRs are failing due to breakage on the main branch, not due to the proposed changes? PR to see if tests can be fixed with trivia patch.

I'm not certain if this is because PRs from external repos need some additional configuration (if so, I'm not finding docs on setup), or if because an update to js-yaml landed and broke unit tests.

I see the affected files are in a directory legacy/ which might mean they shouldn't be modified?

netlify[bot] commented 11 months ago

Deploy request for lando-core-next pending review.

Visit the deploys page to approve it

Name Link
Latest commit e63168b122704446d8fccbc346f694b5098090ca
xurizaemon commented 11 months ago

Added matching update for yaml.safeDump() => yaml.dump()

There's a failure in the lando.spec.js "Lando should use prexisting instance id if possible" test too, which appears to fail because /tmp/cache exists when trying to create /tmp/cache/id?

Maybe we could use /tmp/cache-${randomInteger} or something to avoid conflicts with other test side-effects?

   1) lando
       #Lando
         should use prexisting instance id if possible:
     Error: EEXIST, file already exists '\\?\D:\tmp\cache'
      at Binding.<anonymous> (node_modules\mock-fs\lib\binding.js:808:13)
      at maybeCallback (node_modules\mock-fs\lib\binding.js:68:19)
      at Binding.mkdir (node_modules\mock-fs\lib\binding.js:805:3)
      at Object.mkdirSync (node:fs:1379:26)
      at mkdirpNativeSync (node_modules\mkdirp\lib\mkdirp-native.js:29:10)
      at Function.mkdirpSync [as sync] (node_modules\mkdirp\index.js:21:7)
      at new Cache (legacy\cache.js:24:12)
      at Object.exports.setupCache (legacy\bootstrap.js:229:17)
      at new Lando (legacy\lando.js:191:28)
      at Context.<anonymous> (test\lando.spec.js:31:21)
      at processImmediate (node:internal/timers:466:21)
pirog commented 11 months ago

@xurizaemon i merged this into https://github.com/lando/core-next/pull/45, not sure what we will end up doing with the legacy tests but until then we should get them to pass