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

Add support for testing whether a value is a constant-cased string #540

Merged
merged 21 commits into from Oct 16, 2022
Merged

Add support for testing whether a value is a constant-cased string #540

merged 21 commits into from Oct 16, 2022

Conversation

Pranavchiku
Copy link
Member

@Pranavchiku Pranavchiku commented Oct 11, 2022

Resolves #534.

Checklist

  • Add readme.md
  • Add package.json
  • Add lib
  • Add examples
  • Add docs
  • Add etc
  • Add bin
  • Add benchmark
  • Add test

@kgryte

@Pranavchiku
Copy link
Member Author

Pranavchiku commented Oct 11, 2022

I don't have the access to add assignees for the PR :/

Edit: Thanks :)

@kgryte
Copy link
Member

kgryte commented Oct 11, 2022

@Pranavchiku Sorry about that. This is the first time I'm trying this out. Just assigned. Let's see what happens.

@kgryte
Copy link
Member

kgryte commented Oct 11, 2022

Looks like the workflow is stuck in a queue, so we'll need to wait a while for it to kick off. How long we'll have to wait is TBD.

@kgryte kgryte added the Feature Issue or pull request for adding a new feature. label Oct 11, 2022
@Pranavchiku
Copy link
Member Author

Looks like the workflow is stuck in a queue, so we'll need to wait a while for it to kick off. How long we'll have to wait is TBD.

It might get failed if stuck there for long time ( I faced this issue once at lfortran )

@Planeshifter
Copy link
Member

Okay, so there are currently problems with the AI scaffolding workflow when triggered from a fork, which we will need to look into. Apologies!

As a temporary workaround, I have created a assert/is-constantcase branch with your README.md and the files: #541

Feel free to either checkout that branch or copy the files over into this PR's branch.

@Pranavchiku
Copy link
Member Author

Pranavchiku commented Oct 11, 2022

I rebased it over my PR, and added implementation, made a clean build again, there are quite a few bugs like output not correctly predicted, linting errors, and a few things will be needed to fix before I can push those changes in this PR, will try to get that done as soon as possible :)

@kgryte
Copy link
Member

kgryte commented Oct 11, 2022

Thanks for trying this out @Pranavchiku!

@Pranavchiku
Copy link
Member Author

Sorry for delaying it, will try to get the things up as soon as possible.

@Pranavchiku
Copy link
Member Author

so,

tape( 'the command-line interface supports use as a standard stream', opts, function test( t ) {
	var cmd = [
		'printf "BEEP\nboop"',
		'|',
		EXEC_PATH,
		fpath
	];

	exec( cmd.join( ' ' ), done );

	function done( error, stdout, stderr ) {
		if ( error ) {
			t.fail( error.message );
		} else {
			t.strictEqual( stdout.toString(), 'true\ntrue\n', 'expected value' );
			t.strictEqual( stderr.toString(), '', 'does not print to `stderr`' );
		}
		t.end();
	}
});

This test is not passing in test.cli.js, idk the reason, pasting the logs below

  the command-line interface supports use as a standard stream


    ✖ expected value
    -----------------
      operator: equal
      expected: 'false\nfalse\n'
      actual:   'false\n'
      at: done (/home/pranavchiku/stdlib/lib/node_modules/@stdlib/assert/is-constantcase/test/test.cli.js:179:6)

    ✔ does not print to `stderr`
    ```

@kgryte
Copy link
Member

kgryte commented Oct 16, 2022

@Pranavchiku Left a comment. You may need to merge in changes from upstream. Currently, this PR is a bit "dirty" in that it contains changes/files which are already present in the upstream repo.

@Pranavchiku
Copy link
Member Author

Yeah I will do that.

@Pranavchiku
Copy link
Member Author

Pranavchiku commented Oct 16, 2022

I observed few things on the files created by stdlib-bot

  1. Ending the file with two extra lines
  2. Using extra line after use-strict
  3. Tests logic are not great
  4. test/fixtures file is not created

Rest everything was very perfect!

@kgryte
Copy link
Member

kgryte commented Oct 16, 2022

@Pranavchiku That's great feedback! Thanks! cc @Planeshifter

@Pranavchiku
Copy link
Member Author

I am currently getting a small error in test.cli.js, I will resolve that, polish the PR and then push changes as soon as possible :)

@kgryte
Copy link
Member

kgryte commented Oct 16, 2022

@Pranavchiku Awesome! Thanks for working on this!

@Pranavchiku
Copy link
Member Author

@kgryte you can have a final review.

Copy link
Member

@kgryte kgryte left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Thanks, @Pranavchiku!

@kgryte kgryte merged commit 4b64ebb into stdlib-js:develop Oct 16, 2022
@Planeshifter
Copy link
Member

I observed few things on the files created by stdlib-bot

  1. Ending the file with two extra lines
  2. Using extra line after use-strict
  3. Tests logic are not great
  4. test/fixtures file is not created

Rest everything was very perfect!

Thank you @Pranavchiku for the feedback. 1., 2., and 4. will be fixed in the next iteration of the scaffolding tool. We will keep iterating to hopefully get to a very smooth development experience in the near future.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Feature Issue or pull request for adding a new feature.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[RFC]: add supporting for testing whether a value is a constant-cased string
4 participants