add test for commandline

[WangGithub0]

write test for src/utils/commandline.py file, coverage from 70%->84%

closes #27

[Amnish04]

Hi Yumei, the logic for your unit test looks good. But it is not being collected or run by pytest since its not part of a function/method.

You'll need to add it to a method instead of directly putting in the class like so.

Also, could you please make a separate file for this class, since the pattern is to have a test file for every source file. In your case, it would be tests/utils/test_commandline.py. And the root class name would be TestCommandline. You can put all your test logic in a method for this class.

Lastly, I just noticed that the integration test is failing when running from a pull request. This should be from my side, will check now

Just found that the integration test is being interupted by the commandline test you added. It runs fine without it, we need to find out why.

[Amnish04]

Hi Yumei, the logic for your unit test looks good. But it is not being collected or run by pytest since its not part of a function/method.

You'll need to add it to a method instead of directly putting in the class like so.

Also, could you please make a separate file for this class, since the pattern is to have a test file for every source file.
In your case, it would be tests/utils/test_commandline.py.
And the root class name would be TestCommandline. You can put all your test logic in a method for this class.

[WangGithub0]

hi @Amnish04, thanks for your comments, I've already revise my code, help me view please.

[Amnish04]

hi @Amnish04, thanks for your comments, I've already revise my code, help me view please.

@WangGithub0 Thanks for the changes.
I see there are a couple of lint errors with your imports which is failing the CI.

Fixing those should run the tests step.

[Amnish04]

@WangGithub0 Hi Yumei, I think the change to sys.argv in your test for commandline arguments is being carried on to other tests as well, hence interfering with the integration test run.

I would suggest storing the original state of that system variable before patching it, and then restore that state once you're done with your tests like this.

# Save the original sys.argv
original_argv = sys.argv.copy()

# Apply the patch to sys.argv to mimic command-line arguments
with patch.object(sys, "argv", ["commandline.py"] + arguments):
    # Create an instance of the CommandlineParser
    parser = CommandlineParser()

    # Get the parsed arguments
    args = parser.get_args()

    # Assert that the arguments are correctly parsed
    assert args.config == "config.toml"
    assert args.input_path == "input.md"
    assert args.output == "./dist/til"
    assert args.version

# After the block, revert sys.argv to its original state
sys.argv = original_argv

[Amnish04]

@WangGithub0
Found the problem!

My implementation of CommandlineParser was to only have one instance of the class in a single run.

which is why the new arguments are never parsed and loaded

Changing that to normal __init__ fixes the issue.

[Amnish04]

@WangGithub0 Just pushed the fix fbd0223

You can pull latest from master

[WangGithub0]

@WangGithub0 Just pushed the fix fbd0223

You can pull latest from master

will the test run again automatically?

[Amnish04]

@WangGithub0 Just pushed the fix fbd0223
You can pull latest from master

will the test run again automatically?

Yes

[Amnish04]

@WangGithub0 Just pushed the fix fbd0223
You can pull latest from master

will the test run again automatically?

@WangGithub0
You'll have to pull the latest from upstream master and push to your branch on origin for the workflow to pass.

[Amnish04]

@WangGithub0 Thanks for the contribution!