Add unit tests for the toolchain checks

RESOLVED FIXED in Firefox 48

Status

defect
RESOLVED FIXED
3 years ago
a year ago

People

(Reporter: glandium, Assigned: glandium)

Tracking

(Blocks 2 bugs)

unspecified
mozilla48
Dependency tree / graph

Firefox Tracking Flags

(firefox48 fixed)

Details

Attachments

(10 attachments)

58 bytes, text/x-review-board-request
chmanchester
: review+
Details
58 bytes, text/x-review-board-request
chmanchester
: review+
Details
58 bytes, text/x-review-board-request
chmanchester
: review+
Details
58 bytes, text/x-review-board-request
chmanchester
: review+
Details
58 bytes, text/x-review-board-request
chmanchester
: review+
Details
58 bytes, text/x-review-board-request
chmanchester
: review+
Details
58 bytes, text/x-review-board-request
chmanchester
: review+
Details
58 bytes, text/x-review-board-request
chmanchester
: review+
Details
58 bytes, text/x-review-board-request
chmanchester
: review+
Details
58 bytes, text/x-review-board-request
chmanchester
: review+
Details
Assignee

Description

3 years ago
No description provided.
Assignee

Updated

3 years ago
Blocks: 1264609
Assignee

Updated

3 years ago
Blocks: 1266348
Assignee

Comment 1

3 years ago
Review commit: https://reviewboard.mozilla.org/r/48027/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/48027/
Attachment #8743768 - Flags: review?(cmanchester)
Attachment #8743769 - Flags: review?(cmanchester)
Attachment #8743770 - Flags: review?(cmanchester)
Attachment #8743771 - Flags: review?(cmanchester)
Attachment #8743772 - Flags: review?(cmanchester)
Attachment #8743773 - Flags: review?(cmanchester)
Attachment #8743774 - Flags: review?(cmanchester)
Attachment #8743775 - Flags: review?(cmanchester)
Attachment #8743776 - Flags: review?(cmanchester)
Assignee

Comment 5

3 years ago
This allows unit tests to override the shell check, and is compatible with what
autoconf actually does.

Review commit: https://reviewboard.mozilla.org/r/48035/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/48035/
Assignee

Comment 8

3 years ago
When configure unit tests use an empty mozconfig, instead of creating an
empty temporary file, use an empty mozconfig from the source directory.

Review commit: https://reviewboard.mozilla.org/r/48041/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/48041/
Assignee

Comment 10

3 years ago
Review commit: https://reviewboard.mozilla.org/r/48055/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/48055/
Attachment #8743768 - Attachment description: MozReview Request: Bug 1266343 - Avoid _apply_imports happening twice for the same function. r?chmanchester → MozReview Request: Bug 1266343 - Use mozbuild.util.exec_ in the various configure tests. r?chmanchester
Attachment #8743769 - Attachment description: MozReview Request: Bug 1266343 - Change ConfigureSandbox._apply_imports such that it becomes easy to override imports in unit tests. r?chmanchester → MozReview Request: Bug 1266343 - Avoid _apply_imports happening twice for the same function. r?chmanchester
Attachment #8743770 - Attachment description: MozReview Request: Bug 1266343 - Change FindProgramSandbox to override which.which instead of replacing find_program. r?chmanchester → MozReview Request: Bug 1266343 - Change ConfigureSandbox._apply_imports such that it becomes easy to override imports in unit tests. r?chmanchester
Attachment #8743771 - Attachment description: MozReview Request: Bug 1266343 - Move ConfigureTestSandbox to a common module for all configure tests. r?chmanchester → MozReview Request: Bug 1266343 - Change FindProgramSandbox to override which.which instead of replacing find_program. r?chmanchester
Attachment #8743772 - Attachment description: MozReview Request: Bug 1266343 - Add CONFIG_SHELL to allow to set a POSIX shell that is not sh. r?chmanchester → MozReview Request: Bug 1266343 - Move ConfigureTestSandbox to a common module for all configure tests. r?chmanchester
Attachment #8743773 - Attachment description: MozReview Request: Bug 1266343 - Extend the ConfigureTestSandbox to hook subprocess.Popen. r?chmanchester → MozReview Request: Bug 1266343 - Add CONFIG_SHELL to allow to set a POSIX shell that is not sh. r?chmanchester
Attachment #8743774 - Attachment description: MozReview Request: Bug 1266343 - Create a base class for configure tests, and use it for TestMozConfigure. r?chmanchester → MozReview Request: Bug 1266343 - Extend the ConfigureTestSandbox to hook subprocess.Popen. r?chmanchester
Attachment #8743775 - Attachment description: MozReview Request: Bug 1266343 - Use an empty mozconfig from the source directory. r?chmanchester → MozReview Request: Bug 1266343 - Create a base class for configure tests, and use it for TestMozConfigure. r?chmanchester
Attachment #8743776 - Attachment description: MozReview Request: Bug 1266343 - Add unit tests for the toolchain checks. r?chmanchester → MozReview Request: Bug 1266343 - Use an empty mozconfig from the source directory. r?chmanchester
Attachment #8743794 - Flags: review?(cmanchester)
Assignee

Comment 11

3 years ago
Comment on attachment 8743768 [details]
MozReview Request: Bug 1266343 - Use mozbuild.util.exec_ in the various configure tests. r?chmanchester

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/48027/diff/1-2/
Assignee

Comment 12

3 years ago
Comment on attachment 8743769 [details]
MozReview Request: Bug 1266343 - Avoid _apply_imports happening twice for the same function. r?chmanchester

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/48029/diff/1-2/
Assignee

Comment 13

3 years ago
Comment on attachment 8743770 [details]
MozReview Request: Bug 1266343 - Change ConfigureSandbox._apply_imports such that it becomes easy to override imports in unit tests. r?chmanchester

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/48031/diff/1-2/
Assignee

Comment 14

3 years ago
Comment on attachment 8743771 [details]
MozReview Request: Bug 1266343 - Change FindProgramSandbox to override which.which instead of replacing find_program. r?chmanchester

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/48033/diff/1-2/
Assignee

Comment 15

3 years ago
Comment on attachment 8743772 [details]
MozReview Request: Bug 1266343 - Move ConfigureTestSandbox to a common module for all configure tests. r?chmanchester

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/48035/diff/1-2/
Assignee

Comment 16

3 years ago
Comment on attachment 8743773 [details]
MozReview Request: Bug 1266343 - Add CONFIG_SHELL to allow to set a POSIX shell that is not sh. r?chmanchester

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/48037/diff/1-2/
Assignee

Comment 17

3 years ago
Comment on attachment 8743774 [details]
MozReview Request: Bug 1266343 - Extend the ConfigureTestSandbox to hook subprocess.Popen. r?chmanchester

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/48039/diff/1-2/
Assignee

Comment 18

3 years ago
Comment on attachment 8743775 [details]
MozReview Request: Bug 1266343 - Create a base class for configure tests, and use it for TestMozConfigure. r?chmanchester

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/48041/diff/1-2/
Assignee

Comment 19

3 years ago
Comment on attachment 8743776 [details]
MozReview Request: Bug 1266343 - Use an empty mozconfig from the source directory. r?chmanchester

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/48043/diff/1-2/
Comment on attachment 8743768 [details]
MozReview Request: Bug 1266343 - Use mozbuild.util.exec_ in the various configure tests. r?chmanchester

https://reviewboard.mozilla.org/r/48027/#review44985
Attachment #8743768 - Flags: review?(cmanchester) → review+
Comment on attachment 8743769 [details]
MozReview Request: Bug 1266343 - Avoid _apply_imports happening twice for the same function. r?chmanchester

https://reviewboard.mozilla.org/r/48029/#review44989
Attachment #8743769 - Flags: review?(cmanchester) → review+
Comment on attachment 8743770 [details]
MozReview Request: Bug 1266343 - Change ConfigureSandbox._apply_imports such that it becomes easy to override imports in unit tests. r?chmanchester

https://reviewboard.mozilla.org/r/48031/#review45005
Attachment #8743770 - Flags: review?(cmanchester) → review+
Comment on attachment 8743771 [details]
MozReview Request: Bug 1266343 - Change FindProgramSandbox to override which.which instead of replacing find_program. r?chmanchester

https://reviewboard.mozilla.org/r/48033/#review45015
Attachment #8743771 - Flags: review?(cmanchester) → review+
Comment on attachment 8743772 [details]
MozReview Request: Bug 1266343 - Move ConfigureTestSandbox to a common module for all configure tests. r?chmanchester

https://reviewboard.mozilla.org/r/48035/#review45017
Attachment #8743772 - Flags: review?(cmanchester) → review+
Comment on attachment 8743773 [details]
MozReview Request: Bug 1266343 - Add CONFIG_SHELL to allow to set a POSIX shell that is not sh. r?chmanchester

https://reviewboard.mozilla.org/r/48037/#review45019
Attachment #8743773 - Flags: review?(cmanchester) → review+
Comment on attachment 8743774 [details]
MozReview Request: Bug 1266343 - Extend the ConfigureTestSandbox to hook subprocess.Popen. r?chmanchester

https://reviewboard.mozilla.org/r/48039/#review45027
Attachment #8743774 - Flags: review?(cmanchester) → review+
Comment on attachment 8743775 [details]
MozReview Request: Bug 1266343 - Create a base class for configure tests, and use it for TestMozConfigure. r?chmanchester

https://reviewboard.mozilla.org/r/48041/#review45029
Attachment #8743775 - Flags: review?(cmanchester) → review+
Attachment #8743776 - Flags: review?(cmanchester) → review+
Comment on attachment 8743776 [details]
MozReview Request: Bug 1266343 - Use an empty mozconfig from the source directory. r?chmanchester

https://reviewboard.mozilla.org/r/48043/#review45031
Comment on attachment 8743794 [details]
MozReview Request: Bug 1266343 - Add unit tests for the toolchain checks. r?chmanchester

https://reviewboard.mozilla.org/r/48055/#review45045

::: python/mozbuild/mozbuild/test/configure/test_toolchain_configure.py:498
(Diff revision 1)
> +                self.out.truncate(0)
> +                compiler = sandbox._value_for(sandbox[var])
> +                # Add var on both ends to make it clear which of the
> +                # variables is failing the test when that happens.
> +                self.assertEquals((var, compiler.__dict__), (var, result))
> +            except SystemExit as e:

Unused `e`.
Attachment #8743794 - Flags: review?(cmanchester) → review+
Assignee

Updated

3 years ago
Blocks: 1270446

Updated

a year ago
Product: Core → Firefox Build System
You need to log in before you can comment on or make changes to this bug.