[Reopen] Read args from file, merge args and pass to the cmd.

[vivasvan1]

/claim #191

closes #191

Hi @jdegoes,

Hi have attempted to resolve your review comments on the previous PR

Please let me know if more changes are required.

Working Screenshots:

---

References:

1. Old PR: https://github.com/zio/zio-cli/pull/278

P.S. : I dont have permission to reopen the previous PR so created a fresh one.

[CLAassistant]

All committers have signed the CLA.

[vivasvan1]

Also could i please get provide some guidance in fixing CI/CD error. I am new to this ecosystem and unclear on what exactly the errors are.

[jdegoes]

[error] Modules were resolved with conflicting cross-version suffixes in ProjectRef(uri("file:/home/runner/work/zio-cli/zio-cli/"), "zioCliJVM"):
[error]    org.scala-lang.modules:scala-collection-compat _3, _2.13
[error] java.lang.RuntimeException: Conflicting cross-version suffixes in: org.scala-lang.modules:scala-collection-compat

This error is related to Scala JS, to use of a library scala-collection-compat. Probably version numbers for the Scala compiler(s) need updating to be consistent with each other. I would look in the build file.

[vivasvan1]

Update: 22 Apr 2024: ✅ All Green)) - @jdegoes

Notes:

• JS support can be added later.

---

Update 20 Apr 2024: Tests are also initiallized. Let me know if any changes are required.

---

Looks good! Please just add a test to ensure the behavior is working correctly, and th en we can merge!

@jdegoes Update 19 Apr 2024: Hi there,

First, thank you for your review.

I have commited fix for build errors in JVM and Native.

However, errors are now only in scalaJS. Error are only of this type:

I believe it is JS doesnt support java.io If you know about this could you guide me a little?

Also, I'm working on writing test. should be done soon.

Thank you again :)

[vivasvan1]

@jdegoes Hi there, hope this message finds you well. Reminder: I have fixed the issues you mentioned. Could you please let me know how to proceed? Thanks :)

[pablf]

@vivasvan1 Hi! The tests do not cover the case of a real CliApp. Testing locally the following test suite, it seems to be failing. I don't know if there might be some other issues. Method parse of Command is the one that parse the parameters. I think that the easiest way to solve it would be to modify this method so that it accepts separately the user parameters and the file parameters and then process them knowing to which group each one belongs instead of just concatenating them.

If you could add a test suite similar to this testing the whole process of running the CliApp it would be great!

import zio._
import zio.test._
import zio.test.Assertion._
import java.nio.file.{Files, Paths, Path}
import java.io.IOException
import zio.cli._

object AlternativeSpec extends ZIOSpecDefault {

  val configFileOps: ConfigFilePlatformSpecific = ConfigFileArgsPlatformSpecific

  def spec = suite("FileBasedArgs")(
    test("failing test 1") {
      for {
        // Create Sample config files
        cwd     <- ZIO.succeed(Paths.get(java.lang.System.getProperty("user.dir")))
        _       <- createSampleConfigFiles2(cwd, "someCommand")
        cliApp = CliApp.make(
          name = "cliApp",
          version = "0",
          summary = HelpDoc.Span.empty,
          command = Command("someCommand", Options.text("opt")),
        ) {
          case text: String => ZIO.succeed(text)
        }
        res <- cliApp.run(List("--opt", "inputText"))

        // Check if the func checkAndGetOptionsFilePaths can

        _ <- cleanUpSampleConfigFiles2(cwd, "someCommand")

      } yield assertTrue(res == Some("inputText"))
    },
    test("failing test 2") {
      for {
        // Create Sample config files
        cwd     <- ZIO.succeed(Paths.get(java.lang.System.getProperty("user.dir")))
        command = "someCommandB"
        _       <- createSampleConfigFiles2(cwd, command)
        cliApp = CliApp.make(
          name = "cliApp",
          version = "0",
          summary = HelpDoc.Span.empty,
          command = Command(command, Options.text("opt"), Args.integer("num")),
        ) {
          case text => ZIO.succeed(text)
        }
        res <- cliApp.run(List("--opt", "inputText", "4"))

        // Check if the func checkAndGetOptionsFilePaths can

        _ <- cleanUpSampleConfigFiles2(cwd, command)

      } yield assertTrue(res == Some(("inputText", BigInt(4))))
    },
    test("failing test 3") {
      for {
        // Create Sample config files
        cwd     <- ZIO.succeed(Paths.get(java.lang.System.getProperty("user.dir")))
        command = "someCommandCLeft"
        command2 = "someCommandCRight"
        _       <- createSampleConfigFiles2(cwd, command)
        cliApp = CliApp.make(
          name = "cliApp",
          version = "0",
          summary = HelpDoc.Span.empty,
          command = Command(command, Options.text("opt"), Args.integer("num")) | Command(command2, Options.text("opt"), Args.integer("num")),
        ) {
          case text => ZIO.succeed(text)
        }
        res <- cliApp.run(List(command, "4"))

        
        // Check if the func checkAndGetOptionsFilePaths can

        _ <- cleanUpSampleConfigFiles2(cwd, command)

      } yield assertTrue(res == Some(("fileText", BigInt(4))))
    },
    test("failing test 4") {
      for {
        // Create Sample config files
        cwd     <- ZIO.succeed(Paths.get(java.lang.System.getProperty("user.dir")))
        command = "someCommandD"
        _       <- createSampleConfigFiles2(cwd, command)
        cliApp = CliApp.make(
          name = "cliApp",
          version = "0",
          summary = HelpDoc.Span.empty,
          command = Command(command, Args.integer("num")),
        ) {
          case text => ZIO.succeed(text)
        }
        res <- cliApp.run(List("4"))

        // Check if the func checkAndGetOptionsFilePaths can

        _ <- cleanUpSampleConfigFiles2(cwd, command)

      } yield assertTrue(res == Some(BigInt(4)))
    },
  )

  def createSampleConfigFiles2(cwd: Path, command: String = "testApp"): IO[IOException, Unit] =
    ZIO.attempt {
      Files.write(Paths.get(cwd.toString(), s".$command"), java.util.Arrays.asList("--opt=fileText"));

      ()
    }.refineToOrDie[IOException]

  def cleanUpSampleConfigFiles2(cwd: Path, command: String = "testApp"): IO[IOException, Unit] =
    ZIO.attempt {
      Files.delete(Paths.get(cwd.toString(), s".$command"));

      ()
    }.refineToOrDie[IOException]
}

[vivasvan1]

@pablf Hi there,

I have added the test case you asked for. Where the functionality is being tested on the full wc app. Let me know if any futher changes are required.

I am not sure why your local is failing. Could you provide more details. i tried to understand the parse function but as i am new to zio, scala it felt bit overwhelming.

Thanks, Vivasvan

[vivasvan1]

@Kalin-Rudnicki Wow, thank you so much for all the comments, I really appreciate your help 🙏.

I will look into this asap. This seems really helpful.

[pablf]

@vivasvan1 If I remember correctly, the tests contained in the code of my last comment should all pass if the implementation of the feature were correct, so if you copy that code into zio-cli/jvm/src/test/scala/zio/cli/AlternativeSpec.scala it should be green. It's great testing now on CliApp as you have changed, but I only see one use case tested. I think this is the problem. My sample test should be working (correct me if I'm wrong please) so, would you mind adding it exactly the same to your PR? If it passed, maybe there would be some other test that should be working and isn't, but I think this would be a good start.

If you have any doubt, ask me please! Or maybe you have already tried adding the AlternativeSpec to your build and was passing? Given that it's already written and has tests that should be passing, it should be included in the PR.