Skip to content

feat(codegen): pytest support in codegen #13746

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

Merged
merged 5 commits into from
May 4, 2022
Merged

feat(codegen): pytest support in codegen #13746

merged 5 commits into from
May 4, 2022

Conversation

roniemartinez
Copy link
Contributor

@roniemartinez roniemartinez commented Apr 25, 2022

This is a proposal to add support for pytest in the code generator

NOTE:

image

This is a proposal to add support for pytest in the code generator
@pavelfeldman
Copy link
Member

I'm supportive for adding this kind of the codegen. I would not copy and paste though - take a look at a sibling javascript.ts with its _isTest variable. You merely need to adjust the header and footer based on whether you generate the test, and the rest of the logic will stay the same.

@roniemartinez
Copy link
Contributor Author

@pavelfeldman I updated in 56ebcaf based on your recommendation. Should I also add tests? The Javascript test does not have this either.

@roniemartinez roniemartinez marked this pull request as ready for review April 26, 2022 08:01
@dgozman
Copy link
Contributor

dgozman commented Apr 27, 2022

Should I also add tests? The Javascript test does not have this either.

Take a look at this file - it is testing the javascript implementation.

Copy link
Contributor

@dgozman dgozman left a comment

Choose a reason for hiding this comment

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

This looks great! If you would make device emulation configurable, that would be just wonderful! If you don't have cycles for that, just let us know and we'll follow up.

Copy link
Contributor

@dgozman dgozman left a comment

Choose a reason for hiding this comment

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

Thank you for the PR!

constructor(isAsync: boolean) {
this.id = isAsync ? 'python-async' : 'python';
this.fileName = isAsync ? 'Python Async' : 'Python';
constructor(isAsync: boolean, isTest: boolean) {
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
constructor(isAsync: boolean, isTest: boolean) {
constructor(isAsync: boolean, isPyTest: boolean) {

this._isAsync = isAsync;
this._isTest = isTest;
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
this._isTest = isTest;
this._isPyTest = isPyTest;

Comment on lines +197 to +199
if (this._isTest) {
return '';
} else if (this._isAsync) {
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
if (this._isTest) {
return '';
} else if (this._isAsync) {
if (this._isTest)
return '';
if (this._isAsync) {

});

test('should save the codegen output to a file if specified', async ({ runCLI }, testInfo) => {
const tmpFile = testInfo.outputPath('script.js');
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
const tmpFile = testInfo.outputPath('script.js');
const tmpFile = testInfo.outputPath('test_foo.py');

nit

@dgozman dgozman merged commit 2c597f0 into microsoft:main May 4, 2022
@roniemartinez roniemartinez deleted the pytest-codegen branch May 4, 2022 08:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants