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

test: set module loading error for aix #14511

Closed
wants to merge 1 commit into from
Closed

test: set module loading error for aix #14511

wants to merge 1 commit into from

Conversation

prakaashkpk
Copy link
Contributor

In test/parallel/test-module-loading-error.js, an attempt is made to
load a text file as a native executable. This gives error messages in a
platform specific manner.

AIX was not included in the list of platforms. This fix introduces
the AIX error messages.

Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • commit message follows commit guidelines
Affected core subsystem(s)

@nodejs-github-bot nodejs-github-bot added the test Issues and PRs related to the tests. label Jul 27, 2017
@gireeshpunathil gireeshpunathil added the aix Issues and PRs related to the AIX platform. label Jul 27, 2017
Copy link
Member

@gireeshpunathil gireeshpunathil left a comment

Choose a reason for hiding this comment

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

Thanks @prakaashkpk for contributing!

Copy link
Member

@tniessen tniessen left a comment

Choose a reason for hiding this comment

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

Code looks good, even though I cannot confirm the correctness for AIX.

@gireeshpunathil
Copy link
Member

@gireeshpunathil
Copy link
Member

previous CI was either hung or abandoned. Issued a fresh one.
CI: https://ci.nodejs.org/job/node-test-pull-request/9447/

@@ -28,7 +28,8 @@ const errorMessagesByPlatform = {
win32: ['%1 is not a valid Win32 application'],
linux: ['file too short', 'Exec format error'],
sunos: ['unknown file type', 'not an ELF file'],
darwin: ['file too short']
darwin: ['file too short'],
aix: ['Cannot load module', 'Cannot run a file that does not have a valid format.']
Copy link
Member

Choose a reason for hiding this comment

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

This line exceeds 80 characters, please format the code according to our style guidelines.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I have made the changes accordingly. Please review.

@gibfahn
Copy link
Member

gibfahn commented Aug 5, 2017

cc/ @nodejs/platform-aix

@jasnell
Copy link
Member

jasnell commented Aug 8, 2017

In test/parallel/test-module-loading-error.js, an attempt is made to
load a text file as a native executable. This gives error messages in a
platform specific manner.

AIX was not included in the list of platforms. This fix introduces
the AIX error messages.
@tniessen
Copy link
Member

tniessen pushed a commit that referenced this pull request Aug 10, 2017
In test/parallel/test-module-loading-error.js, an attempt is made to
load a text file as a native executable. This results in an error
message in a platform specific manner.

AIX was not included in the list of platforms. This fix introduces
the AIX error messages.

PR-URL: #14511
Reviewed-By: Tobias Nießen <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Gireesh Punathil <[email protected]>
Reviewed-By: Gibson Fahnestock <[email protected]>
@tniessen
Copy link
Member

Landed in 8172c8a, thank you for your contribution! 🎉

The trailing whitespace caused the CI linter failure. In the future, you might want to use an editor with editorconfig support, this will automatically fix such problems.

@tniessen tniessen closed this Aug 10, 2017
addaleax pushed a commit that referenced this pull request Aug 10, 2017
In test/parallel/test-module-loading-error.js, an attempt is made to
load a text file as a native executable. This results in an error
message in a platform specific manner.

AIX was not included in the list of platforms. This fix introduces
the AIX error messages.

PR-URL: #14511
Reviewed-By: Tobias Nießen <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Gireesh Punathil <[email protected]>
Reviewed-By: Gibson Fahnestock <[email protected]>
@addaleax addaleax mentioned this pull request Aug 13, 2017
@prakaashkpk prakaashkpk deleted the aix-error-message branch August 22, 2017 04:26
MylesBorins pushed a commit that referenced this pull request Sep 19, 2017
In test/parallel/test-module-loading-error.js, an attempt is made to
load a text file as a native executable. This results in an error
message in a platform specific manner.

AIX was not included in the list of platforms. This fix introduces
the AIX error messages.

PR-URL: #14511
Reviewed-By: Tobias Nießen <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Gireesh Punathil <[email protected]>
Reviewed-By: Gibson Fahnestock <[email protected]>
@MylesBorins MylesBorins mentioned this pull request Sep 20, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
aix Issues and PRs related to the AIX platform. test Issues and PRs related to the tests.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants